From 98700cfcc791d498cb94e71c4fe400a7bd93078e Mon Sep 17 00:00:00 2001 From: CodeJuggernaut <81205671+CodeJuggernaut@users.noreply.github.com> Date: Wed, 15 Jul 2026 02:20:31 -0700 Subject: [PATCH] feat: marquee select and Ctrl/Cmd+A in the Media Assets panel (#13323) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Adds marquee (rubber-band) multi-select and Ctrl/Cmd+A select-all to the Media Assets panel, clips the canvas drag-selection rectangle to the canvas panel, and turns on live (real-time) node-graph rubber-band selection by default. ## Changes - **Marquee select** — rubber-band drag from empty grid space selects the covered cards; hold Ctrl/Cmd to start a marquee from over a card; Ctrl/Cmd or Shift alone makes the marquee additive to the current selection, Ctrl/Cmd+Shift subtracts the covered cards from it (designer-approved), and no modifier replaces it. Cards update their selected state live during the drag. - **Ctrl/Cmd+A** — selects all loaded assets when the pointer is over the panel, otherwise falls through to the canvas (select all nodes). It `stopImmediatePropagation`s so a panel select-all never also fires the global node select-all, and it yields while an `aria-modal` dialog is open or a text input is focused. - **Select-all recovers after "deselect all"** — the shortcut was gated only on `useElementHover`, which latched stale when the floating selection bar under the cursor unmounted on deselect. It now also checks the live pointer position against the panel rect, so a second Ctrl/Cmd+A right after deselecting no longer falls through to the browser's native page select-all. - **Canvas rectangle clip** — the canvas drag-selection rectangle is clamped to the canvas panel bounds (`SelectionRectangle.vue`, display-only). - **Graph live selection on by default** — flips the existing `Comfy.Graph.LiveSelection` setting's default to on, so node-graph rubber-band selection updates in real time during the drag (matching the assets panel) instead of committing only on mouse-up. The behavior was already implemented behind the setting; this changes only the default, and users with an explicit value keep it. - **Robustness/UX** — the pointer is captured on drag-engage rather than on press (so a Ctrl/Cmd-click on a card isn't hijacked); no global `document.body.userSelect` mutation (replaced by a panel-scoped `selectstart` guard); the marquee overlay uses the semantic `primary-background` token; post-drag click-suppression auto-resets so a cancelled drag can't swallow a later click; `setPointerCapture` is wrapped in try/catch; a Ctrl/Cmd-held card `dragstart` is cancelled so no native ghost-drag image appears. - **Breaking**: none — `useAssetSelection` is extended additively (new `setSelectedIds` helper, nothing removed or altered) and the new composable exposes only `{ marqueeStyle }`. - **Dependencies**: none. ## Review Focus - **`SelectionRectangle.vue`** is shared canvas code; the change is display-only (clamps the rectangle to the panel; no node-selection behavior change). - **`coreSettings.ts`** — a one-line `Comfy.Graph.LiveSelection` default flip is the only change that affects graph behavior; the live-select code path itself is pre-existing. - **`useAssetGridSelection.ts`** — listener lifecycle/teardown, the panel-scoped `selectstart` guard, the click-suppression timer, the capture-on-drag-engage logic, the pointer-position select-all fallback, and the subtractive-mode snapshot at pointerdown. - **Ctrl/Cmd+A routing** — panel hover (or a live pointer inside the panel) gates select-all vs. the canvas, and `stopImmediatePropagation` prevents double-handling. - Pure geometry/selection logic is extracted into `marqueeSelectionUtil.ts` and unit-tested in isolation (`RectEdges` is `Pick`, the DOM edge subset); `MediaAssetCard.dragStart` keeps `main`'s `display_name` payload. Relates to Linear **FE-910**. ## Testing - **Unit:** `useAssetGridSelection` (39 cases — marquee selection, additive/replace, subtractive Ctrl/Cmd+Shift (incl. the macOS Cmd variant and a shrink-restore drag), interactive-element + list-view guards, `selectstart` scoping, click-suppression auto-reset, pointer-capture-throw and capture-on-drag-not-press, modal-aware Ctrl/Cmd+A, non-propagation, and the deselect-recovery pointer-in-panel path), plus `MediaAssetCard`, `marqueeSelectionUtil` (11 cases incl. subtractive, and a 5-case fast-check property suite pinning the additive/subtractive set invariants), `SelectionRectangle`, `useAssetSelection`, and `mathUtil`. - **E2E (`assetsSidebarTab.spec.ts`):** 10 Playwright scenarios running in CI — Ctrl/Cmd+A hover vs. canvas; a marquee from the panel header; a modifier-held additive marquee; a Ctrl/Cmd+Shift subtractive marquee; Ctrl/Cmd-drag from a card and within a single card; Ctrl/Cmd+A ignored in a focused search box and under an aria-modal dialog; and a drag from the search box not marquee-selecting. The empty-space marquee path is covered by the panel-header scenario plus the unit suite (a dedicated empty-space e2e could not run headless without a local backend and was dropped as redundant). ## Future work - **Escape key** — not handled by the marquee/select-all flow yet (the composable handles only Ctrl/Cmd+A). Follow-up: press Escape to cancel an in-progress marquee drag (abort the rubber-band and restore the pre-drag selection) and to clear the current selection while the panel has focus. - **Ctrl+A across pagination** — select-all covers the loaded assets only (confirmed as the intended behavior with design); a load-all-then-select variant can follow if needed. ## Demo https://github.com/user-attachments/assets/3841bf3c-db75-4229-a5e7-fb363b4882d6 --- .../fixtures/components/SidebarTab.ts | 4 + .../tests/sidebar/assetsSidebarTab.spec.ts | 252 ++++++ src/components/graph/GraphCanvas.vue | 11 +- .../graph/SelectionRectangle.test.ts | 106 +++ src/components/graph/SelectionRectangle.vue | 51 +- .../sidebar/tabs/AssetsSidebarTab.vue | 59 +- .../assets/components/MediaAssetCard.test.ts | 118 +++ .../assets/components/MediaAssetCard.vue | 6 + .../composables/useAssetGridSelection.test.ts | 800 ++++++++++++++++++ .../composables/useAssetGridSelection.ts | 250 ++++++ .../composables/useAssetSelection.test.ts | 30 + .../assets/composables/useAssetSelection.ts | 14 + .../marqueeSelectionUtil.property.test.ts | 93 ++ .../assets/utils/marqueeSelectionUtil.test.ts | 93 ++ .../assets/utils/marqueeSelectionUtil.ts | 48 ++ .../settings/constants/coreSettings.ts | 2 +- src/utils/mathUtil.test.ts | 26 + src/utils/mathUtil.ts | 24 + 18 files changed, 1961 insertions(+), 26 deletions(-) create mode 100644 src/components/graph/SelectionRectangle.test.ts create mode 100644 src/platform/assets/components/MediaAssetCard.test.ts create mode 100644 src/platform/assets/composables/useAssetGridSelection.test.ts create mode 100644 src/platform/assets/composables/useAssetGridSelection.ts create mode 100644 src/platform/assets/utils/marqueeSelectionUtil.property.test.ts create mode 100644 src/platform/assets/utils/marqueeSelectionUtil.test.ts create mode 100644 src/platform/assets/utils/marqueeSelectionUtil.ts diff --git a/browser_tests/fixtures/components/SidebarTab.ts b/browser_tests/fixtures/components/SidebarTab.ts index 3db6d04bc1..bcece779a3 100644 --- a/browser_tests/fixtures/components/SidebarTab.ts +++ b/browser_tests/fixtures/components/SidebarTab.ts @@ -322,6 +322,9 @@ export class AssetsSidebarTab extends SidebarTab { // --- Folder view --- public readonly backToAssetsButton: Locator + // --- Panel chrome --- + public readonly panelHeader: Locator + // --- Loading --- public readonly skeletonLoaders: Locator @@ -358,6 +361,7 @@ export class AssetsSidebarTab extends SidebarTab { this.deleteSelectedButton = page.getByTestId('assets-delete-selected') this.downloadSelectedButton = page.getByTestId('assets-download-selected') this.backToAssetsButton = page.getByText('Back to all assets') + this.panelHeader = page.locator('.comfy-vue-side-bar-header') this.skeletonLoaders = page.locator( '.sidebar-content-container .animate-pulse' ) diff --git a/browser_tests/tests/sidebar/assetsSidebarTab.spec.ts b/browser_tests/tests/sidebar/assetsSidebarTab.spec.ts index becfffc4cb..30fb7ec8af 100644 --- a/browser_tests/tests/sidebar/assetsSidebarTab.spec.ts +++ b/browser_tests/tests/sidebar/assetsSidebarTab.spec.ts @@ -276,3 +276,255 @@ test.describe('FE-130 assets sidebar route mocks', () => { ) }) }) + +test.describe('FE-910 marquee selection and select all', () => { + test.beforeEach(async ({ jobsRoutes, page, comfyPage }) => { + await jobsRoutes.mockJobsQueue([]) + await jobsRoutes.mockJobsHistory(generatedJobs) + await mockInputFiles(page, ['imported.png']) + await mockViewFiles(page, viewFiles) + await comfyPage.setup() + await comfyPage.menu.assetsTab.open() + }) + + test('Ctrl/Cmd+A selects every asset while the panel is hovered', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + + await expect(tab.assetCards).toHaveCount(2) + + await tab.getAssetCardByName('alpha').hover() + await comfyPage.page.keyboard.press('ControlOrMeta+a') + + await expect(tab.selectedCards).toHaveCount(2) + }) + + test('a marquee that begins in the panel header selects the cards', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + const { page } = comfyPage + + await expect(tab.assetCards).toHaveCount(2) + await expect(tab.selectedCards).toHaveCount(0) + + const header = await tab.panelHeader.boundingBox() + const beta = await tab.getAssetCardByName('beta').boundingBox() + if (!header || !beta) { + throw new Error('panel header or asset card has no layout box') + } + + // Begin the rubber-band in the header (above the grid), then drag down + // across both cards. + await page.mouse.move(header.x + 24, header.y + 20) + await page.mouse.down() + await page.mouse.move(beta.x + 8, beta.y + beta.height - 8, { steps: 14 }) + await page.mouse.up() + + await expect(tab.selectedCards).toHaveCount(2) + await expect(tab.selectionFooter).toBeVisible() + }) + + test('Ctrl/Cmd+A leaves assets unselected while the canvas is hovered', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + const { page } = comfyPage + + await expect(tab.assetCards).toHaveCount(2) + + const viewport = page.viewportSize() + if (!viewport) throw new Error('viewport size is unavailable') + + // Hover the canvas (not the panel); Ctrl/Cmd+A must yield to the canvas. + await page.mouse.move(viewport.width - 100, viewport.height / 2) + await page.keyboard.press('ControlOrMeta+a') + + await expect(tab.selectedCards).toHaveCount(0) + }) + + test('a modifier-held marquee adds to the existing selection', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + const { page } = comfyPage + + await expect(tab.assetCards).toHaveCount(2) + + await tab.getAssetCardByName('alpha').click() + await expect(tab.selectedCards).toHaveCount(1) + + const beta = await tab.getAssetCardByName('beta').boundingBox() + if (!beta) throw new Error('beta card has no layout box') + + // Hold a modifier so the marquee is additive, then rubber-band over beta. + await page.keyboard.down('Control') + await page.mouse.move(beta.x + 12, beta.y + 12) + await page.mouse.down() + await page.mouse.move(beta.x + beta.width - 12, beta.y + beta.height - 12, { + steps: 12 + }) + await page.mouse.up() + await page.keyboard.up('Control') + + await expect(tab.selectedCards).toHaveCount(2) + }) + + test('a Ctrl/Cmd+Shift marquee removes the covered cards from the selection', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + const { page } = comfyPage + + await expect(tab.assetCards).toHaveCount(2) + + await tab.getAssetCardByName('alpha').hover() + await page.keyboard.press('ControlOrMeta+a') + await expect(tab.selectedCards).toHaveCount(2) + + const beta = await tab.getAssetCardByName('beta').boundingBox() + if (!beta) throw new Error('beta card has no layout box') + + // Ctrl+Shift makes the marquee subtractive: rubber-band over beta only. + await page.keyboard.down('Control') + await page.keyboard.down('Shift') + await page.mouse.move(beta.x + 12, beta.y + 12) + await page.mouse.down() + await page.mouse.move(beta.x + beta.width - 12, beta.y + beta.height - 12, { + steps: 12 + }) + await page.mouse.up() + await page.keyboard.up('Shift') + await page.keyboard.up('Control') + + await expect(tab.selectedCards).toHaveCount(1) + await expect(tab.getAssetCardByName('alpha')).toHaveAttribute( + 'data-selected', + 'true' + ) + }) + + test('Ctrl/Cmd-dragging from an asset card starts a marquee selection', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + const { page } = comfyPage + + await expect(tab.assetCards).toHaveCount(2) + await expect(tab.selectedCards).toHaveCount(0) + + const alpha = await tab.getAssetCardByName('alpha').boundingBox() + const beta = await tab.getAssetCardByName('beta').boundingBox() + if (!alpha || !beta) throw new Error('asset cards have no layout box') + + // Ctrl bypasses card drag, so a press that begins on a card rubber-bands. + await page.keyboard.down('Control') + await page.mouse.move(alpha.x + alpha.width / 2, alpha.y + alpha.height / 2) + await page.mouse.down() + await page.mouse.move(beta.x + beta.width - 6, beta.y + beta.height - 6, { + steps: 12 + }) + await page.mouse.up() + await page.keyboard.up('Control') + + await expect(tab.selectedCards).toHaveCount(2) + await expect(tab.selectionFooter).toBeVisible() + }) + + test('Ctrl/Cmd-dragging within a single card selects only that card', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + const { page } = comfyPage + + await expect(tab.assetCards).toHaveCount(2) + + const alpha = tab.getAssetCardByName('alpha') + const box = await alpha.boundingBox() + if (!box) throw new Error('alpha card has no layout box') + + const start = { x: box.x + box.width / 2, y: box.y + box.height / 2 } + await page.keyboard.down('Control') + await page.mouse.move(start.x, start.y) + await page.mouse.down() + await page.mouse.move(start.x + 12, start.y + 12, { steps: 4 }) + await page.mouse.up() + await page.keyboard.up('Control') + + await expect(tab.selectedCards).toHaveCount(1) + await expect(alpha).toHaveAttribute('data-selected', 'true') + }) + + test('Ctrl/Cmd+A in the focused search input does not select assets', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + const query = 'alpha' + + await tab.searchInput.fill(query) + await expect(tab.assetCards).toHaveCount(1) + + await tab.searchInput.focus() + await comfyPage.page.keyboard.press('ControlOrMeta+a') + + await expect(tab.selectedCards).toHaveCount(0) + await expect + .poll(() => + tab.searchInput.evaluate((el: HTMLInputElement) => { + return { start: el.selectionStart, end: el.selectionEnd } + }) + ) + .toEqual({ start: 0, end: query.length }) + }) + + test('a drag starting in the search input does not marquee-select assets', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + const { page } = comfyPage + + await expect(tab.assetCards).toHaveCount(2) + + const search = await tab.searchInput.boundingBox() + const beta = await tab.getAssetCardByName('beta').boundingBox() + if (!search || !beta) + throw new Error('search box or card has no layout box') + + await page.mouse.move( + search.x + search.width / 2, + search.y + search.height / 2 + ) + await page.mouse.down() + await page.mouse.move(beta.x + beta.width / 2, beta.y + beta.height / 2, { + steps: 12 + }) + await page.mouse.up() + + await expect(tab.selectedCards).toHaveCount(0) + }) + + test('Ctrl/Cmd+A does not select assets while an aria-modal dialog is open', async ({ + comfyPage + }) => { + const tab = comfyPage.menu.assetsTab + await expect(tab.assetCards).toHaveCount(2) + + await comfyPage.page.evaluate(() => { + const dialog = document.createElement('div') + dialog.id = 'test-modal' + dialog.setAttribute('role', 'dialog') + dialog.setAttribute('aria-modal', 'true') + document.body.appendChild(dialog) + }) + + await tab.getAssetCardByName('alpha').hover() + await comfyPage.page.keyboard.press('ControlOrMeta+a') + + await expect(tab.selectedCards).toHaveCount(0) + + await comfyPage.page.evaluate(() => { + document.getElementById('test-modal')?.remove() + }) + }) +}) diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 0d46429c1f..426e6a44fb 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -39,6 +39,10 @@