Skip to content

feat(media): select every visible asset with Ctrl/Cmd+A or a toolbar button - #501

Closed
mostafasadeghidev wants to merge 7 commits into
CoreBunch:mainfrom
mostafasadeghidev:feat/media-select-all
Closed

mostafasadeghidev wants to merge 7 commits into
CoreBunch:mainfrom
mostafasadeghidev:feat/media-select-all

Conversation

@mostafasadeghidev

Copy link
Copy Markdown
Contributor

Media had no way to select more than a range by hand. Emptying the trash meant shift-clicking from the first row to the last — there was no shortcut and no button, while the Data workspace has both a "Select all rows" header checkbox (DataGridHeaderRow.tsx) and a bulk action bar.

Stacked on #500, which is what gives the resulting selection somewhere to go — a context menu that honours it and a bulk permanent-delete.

What "all" means

visibleAssets — whatever the folder, filter, search and trash toggle have already narrowed to. That is what every file manager means by Select All, and it is the only reading that keeps "select all, then delete permanently" in the trash from reaching live assets.

Two ways in, because they fail differently

Ctrl/Cmd+A is what people try first, but only if they already know it is there. The toolbar button is what tells them, and it carries the count so the scope is visible before the click.

Where the shortcut stands down

Document-level, because the grid is a plain div with no tabindex — a React onKeyDown would only fire while focus happened to sit on a tile. It yields in two cases:

  • inside a text field (input, textarea, contenteditable) — someone typing a filter means select-the-text
  • while any dialog is open — it belongs to whatever that dialog contains; selecting the grid behind a confirmation would change what the confirmed action applies to

Seven tests cover exactly those boundaries, including alertdialog, which is what Dialog renders for the permanent-delete confirmation.

No new workspace API — addToSelection already existed and already keeps the selection-order list in step.

bun test src/__tests__/media/   43 pass, 0 fail
tsc -b, eslint                  clean

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

mostafasadeghidev and others added 7 commits September 5, 2026 13:59
Three reports from one install, all the same shape: an affordance that reads
as if it already honours the selection, and does not.

RIGHT-CLICK IGNORED THE SELECTION. `openContextMenu` stored only the clicked
asset and the delete handler acted on `contextMenu.asset` alone, so
right-clicking one of five selected files and choosing Delete trashed exactly
one and left four selected — no error, nothing to notice.

The fix is the rule the same component already uses forty lines above, in
`handleAssetDragStart`: an item inside the selection acts on the whole
selection, an item outside acts on itself. The menu now shows a "5 files"
header and a "Trash 5 files" label so the scope is visible before the click,
and hides Rename for a multi-selection because there is one name field.

Deliberately NOT adopting the clicked asset into the selection the way the
site explorer does: Media derives its floating windows from the selection
during render — viewer at <= 1, bulk edit at >= 2 — so writing the selection
from a menu opener would pop a window open underneath the menu.

THE TRASH HAD NO BULK DELETE. A selection there offered Restore and nothing
else, leaving "empty the trash" a one-file-at-a-time job through the preview
window. `runPurgeAll` sits beside its Trash and Restore siblings and loops the
same single-id endpoint they do — no server work needed. It confirms first,
counting only the trashed members: `purgeAsset` 400s on a live asset, so a
mixed selection would otherwise promise more than it does.

ESCAPE DID NOT CLOSE THE WINDOWS. Every other overlay in the admin takes it.
These windows overlay the grid they were opened from, and the only way out
was the header's close button.

`useTopmostEscape` carries the settings modal's rule — only the topmost layer
reacts, so a confirmation opened from inside a window owns Escape until it
closes rather than collapsing the stack in one press. `FloatingWindow` uses
it, which covers bulk edit, the upload queue and the agent image preview; the
media viewer renders its own shell, so it wires the hook directly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI caught what the stacking check missed. `AgentImagePreview` is a
`FloatingWindow` too, and its test opens a context menu inside the preview,
presses Escape, and asserts the menu closes while the preview stays open.
With the window listening, one press closed both.

The check only looked for `dialog` and `alertdialog`, and only for elements
that follow the panel in document order. A menu is neither: it carries
`role="menu"` and portals to `document.body`, so it can render before the
panel it belongs to.

An open menu now owns Escape wherever it sits. That is the right rule
regardless of the test — closing a window out from under the menu the user
just opened loses their place.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…button

Media had no way to select more than a range by hand. Emptying the trash
meant shift-clicking from the first row to the last, and there was no
shortcut and no button — while the Data workspace has both a "Select all
rows" header checkbox and a bulk action bar.

"All" is `visibleAssets`: whatever the folder, filter, search and trash
toggle have already narrowed to. That is what every file manager means, and
it is the only reading that keeps "select all, then delete permanently" in
the trash from reaching live assets.

Two ways in, because they fail differently. Ctrl/Cmd+A is what people try
first, but only if they already know it is there; the toolbar button is what
tells them. It carries the count, so the scope is visible before the click.

The shortcut is document-level — the grid is a plain div with no tabindex, so
a React `onKeyDown` would only fire while focus happened to sit on a tile —
and stands down in two cases: inside a text field, where Ctrl/Cmd+A means
select-the-text, and while any dialog is open, where it belongs to whatever
that dialog contains. Selecting the grid behind a confirmation would change
what the confirmed action applies to.

No new workspace API: `addToSelection` already existed and already keeps the
selection order list in step.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…bling it

Two corrections to the commit before this one, both found by using it.

THE SHORTCUT BARELY WORKED. The guard stood down whenever any
`role="dialog"` was in the document — written for confirmation dialogs, but
the media viewer window carries that role too, and selecting a single asset
opens it. So Ctrl/Cmd+A was dead for almost the whole time anyone spends in
Media.

`role="dialog"` was the wrong test. What marks a surface as owning the
keyboard is `aria-modal="true"`: `Dialog` sets it, the floating windows
deliberately do not, because the grid stays usable behind them. The guard now
matches that instead, which is both narrower and the thing it always meant.

THE BUTTON ONLY WENT ONE WAY. Select All with no way back is half a control —
the obvious second press should undo it. It now clears when the selection
already covers every visible asset, reads "None" in that state, and reports
`pressed` so it looks like the toggle it is. Ctrl/Cmd+A follows the same
rule, so the two entry points stay one feature.

"Already covers" is deliberately not an equality check: a selection made
before narrowing the filter can hold ids that are no longer visible, and
those should not stop the button offering to clear.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…dismissed

THE UPLOAD WINDOW COULD NOT BE CLOSED MID-TRANSFER. The close button worked
and an effect immediately undid it: the guard read `active && !uploadQueueOpen`
with BOTH in its dependencies, so every close re-ran it with `open` now false
and reopened the window on the next commit. While files were uploading the
window simply would not go away.

Keying it on the transition into `active` alone fixes it. Closing now hides
the transfer rather than cancelling it, so the toolbar button carries the
count — `Uploads 3/7` — which is what makes dismissing safe rather than
lossy. A failed or cancelled item counts as finished, not in flight, or the
count would stick mid-way forever.

MINIMIZE, in the shared shell so all three windows get it from one place.
Collapsing leaves the title bar and hides the body; the header's other
actions go with it, since they act on content that is no longer visible.

Two decisions worth stating:

Collapsed windows stay WHERE THEY ARE. The position is the user's own —
`useDraggablePanel` persists it — so folding to a corner would discard a
choice they made, and expanding would then have nowhere honest to return to.
With three windows able to collapse, a shared corner would also stack them.

The close button still closes. Turning it into "minimize while busy" would
make a control that does something other than what it says, which is the
same defect as a keycap that animates and ignores the click.

What DOES move is the upload window's DEFAULT position: bottom-left, where a
browser puts its download shelf and Finder its copy progress. Default only —
a stored position still wins, so a window the user has moved stays moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TWO GAPS FOUND BY LOOKING AT THE RESULT.

The image viewer window had no minimize control. `FloatingWindow` grew one,
which covered the upload queue, bulk edit and the agent preview — but this
window builds its own shell and only borrows the drag hook, so it got
nothing. Same shape as the Escape gap earlier, and the same fix: wire the
control directly, with the same per-session scope and the same reason for
staying put.

Dialog body copy rendered at the browser's 16px default while the rest of
the admin runs at 12-14px. `.body` set no `font-size`, and neither does
anything above it — `globals.css` contains no `font-size` rule at all, so
there is no base to inherit. Every existing caller had quietly worked around
it by sizing its own children (ImportHtmlModal has five such rules,
SchedulePublishDialog one), which is why it went unnoticed until a dialog
shipped a bare <p>: the delete confirmation's text sat visibly larger than
the panel behind it.

Fixed in the primitive rather than in the caller, since the next bare <p>
would land in the same hole. `--text-m` and `--text-muted` match what
sibling surfaces use for secondary copy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The minimize control hid the body and left the window at full size, so
collapsing it produced a title bar over a tall empty pane rather than a
title bar alone.

`FloatingWindow` did not have this problem because its height comes from a
custom property the component already overrides when collapsed. This window
sets `height` to a fixed length in its own stylesheet, which no amount of
hiding children can shrink.

`height: auto` under `[data-minimized]` lets it size to the header that
remains. `top` keeps clamping against the FULL height on purpose — using the
collapsed height there would let a window pinned near the bottom edge jump
upward as it folds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Sep 6, 2026
…at had no gate

The fork gate stopped at CoreBunch#359. Everything merged onto the stack since —
CoreBunch#497 through CoreBunch#501, and now CoreBunch#505 — was carried with nothing to catch it
being dropped by a merge resolution, which is the exact failure the gate
exists for.

Pinning them turned one up immediately: CoreBunch#498's guard around
`removePluginVersionAssets(…, fromVersion)` is gone, because CoreBunch#359 removed
that call outright — an upgrade now leaves the old version's assets on
disk so published pages keep resolving. What survives of CoreBunch#498 is the
rollback guard, so that is what the row pins.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Sep 13, 2026
The conflicts here are history rather than content: the stack took CoreBunch#507's
warning through its own commits, combined with CoreBunch#500 and CoreBunch#501, so both sides
look new to git. Each file was settled by what CoreBunch#507's commits since then
actually changed:

  - MediaCanvas, MediaViewerWindow, the confirmation test, mediaPrefetch —
    untouched by those commits, so the stack's copies already hold all of
    CoreBunch#507 plus the multi-select and minimize work. Kept.
  - contentUsage.ts and its test — the branch-aware lookup is new, so
    CoreBunch#507's version is taken, with the stack's four-argument walker
    (`rootNodeId` scopes hole and loop fragments here).
  - usageWarning copy + tests, the ref schema, docs — merged cleanly.

Before choosing a side, the stack's copies of both test files were checked
against CoreBunch#507's pre-sync versions: identical, so nothing stack-only is lost.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@DavidBabinec

Copy link
Copy Markdown
Contributor

Hi @mostafasadeghidev

Thanks for this, select-all in the media grid is something we want. I am closing the PR in this form though, because it carries much more than the title says.

Besides select-all, the diff includes the upload-queue reopen fix, a new minimizable prop on FloatingWindow with a hand-wired minimize on the viewer, a new default position for the upload queue, and a change to the shared Dialog.module.css that retypes every dialog in the admin. Four unrelated changes in one PR is hard to review, and the Dialog change in particular needs its own discussion. It is also stacked on #500, which has its own open points (the trash-view context menu purges N files with no confirmation, and the tests assert on local copies of the logic instead of the components).

Could you resubmit these as separate PRs on main? Select-all alone, the upload-queue reopen fix, minimize, and the Dialog typography if you still think it's needed. I will get to each of them quickly.

Btw. #335 is merged now, thanks again for that one.

Have a good one!

mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Oct 16, 2026
…ents

Review on CoreBunch#501 named two problems in this PR, both real:

- The trash view's context menu purged a whole selection with no
  confirmation. A purge from the menu now always asks first, through the
  admin's one ConfirmDeleteProvider with `alwaysConfirm` — the
  `confirmBeforeDelete` preference defaults off and governs reversible
  trashing, not this. A single file asks too, worded as CoreBunch#497 words it, so
  whichever of the two lands second resolves to the same code.
- The tests asserted on local copies of the logic. They now render
  MediaCanvas and BulkEditWindow inside the real session and confirm
  providers and drive the menu, the drag and the dialog. Each was checked
  against a deliberately broken component: no confirmation, a menu or a
  drag that ignores the selection, and a bulk purge of the whole selection
  — every one turns the suite red.

Writing those tests found a bug the copies hid: the bulk window's dialog
counted only the trashed members of a mixed selection, but the purge loop
ran over all of them, firing a `purgeAsset` for every live file — an
endpoint that 400s on an asset that was never soft-deleted. The purge now
runs on exactly the files the dialog counts.

The selection rule itself moves to `utils/actionTargets.ts`, and both the
drag and the menu call it, so they cannot drift apart; the test that
compared two copies of it is gone with them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mostafasadeghidev
mostafasadeghidev deleted the feat/media-select-all branch October 16, 2026 18:42
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Oct 16, 2026
CoreBunch#659 is the upload-queue fix split out of CoreBunch#501, rewritten per review:
`useUploadQueueWindow(active)` opens the window on the transition into an
active upload, detected during render, instead of the effect the stack
carried from CoreBunch#501. Same behaviour, no set-state-in-effect disable, and a
test that drives the real hook. The stack's progress label on the toolbar
button (CoreBunch#501, not part of CoreBunch#659) stays, with its tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Oct 16, 2026
…hat render the canvas

The stack already had select-all, extracted into `useMediaSelection`. The
merge also dropped a second copy of it into MediaCanvas — a second
document keydown listener would have selected and then cleared on one
Ctrl/Cmd+A — so that copy is removed and MediaCanvas is unchanged by this
merge. CoreBunch#658's tests, which render MediaCanvas and press the real shortcut,
replace CoreBunch#501's tests of local copies, and pass against the hook unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Oct 16, 2026
…tests

The stack already carried CoreBunch#501's minimize code, identical to CoreBunch#660's; the
two conflicts were lines CoreBunch#660 does not have (CoreBunch#500's useTopmostEscape, the
viewer's purge-confirmation state from CoreBunch#497/CoreBunch#507), kept as they were, and
the merge's second `minimizable` attribute on two windows is dropped. What
this adds is CoreBunch#660's docs and its tests, which render the real windows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mostafasadeghidev added a commit to mostafasadeghidev/Instatic that referenced this pull request Oct 16, 2026
…ch#501 is now three PRs

CoreBunch#335 and CoreBunch#499 move to the landed table (shipped in v0.0.21). CoreBunch#501 was
closed in review and asked to come back split: its rows become CoreBunch#658
(select-all), CoreBunch#659 (upload queue stays closed) and CoreBunch#660 (minimize). The
three parts of CoreBunch#501 that are in none of them — the Uploads progress label,
the queue's bottom-left default, and the Dialog body size — are listed as
carried-but-not-proposed, so the README still says what a server runs.

Also: what an installation needs before moving to plain upstream, since
upstream's 031 repeats an ALTER this branch ran as 025.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mostafasadeghidev

Copy link
Copy Markdown
Contributor Author

Thanks David, that's fair. And thanks for merging #335!

I've split it up and opened each part on its own against main:

#658 for select-all
#659 for the upload queue reopening while an upload is running
#660 for minimizing the floating windows

None of them depend on #500 or on each other anymore, and I rewrote the tests so they render the actual components instead of testing copies of the logic.

I left out the Dialog typography change and the new default position for the upload queue. If you think the dialog font size is worth looking at, I can open that one separately so we can talk about it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants