Skip to content

feat(media): let the multi-selection reach the actions that imply it - #500

Open
mostafasadeghidev wants to merge 2 commits into
CoreBunch:mainfrom
mostafasadeghidev:feat/media-multi-select-actions
Open

feat(media): let the multi-selection reach the actions that imply it#500
mostafasadeghidev wants to merge 2 commits into
CoreBunch:mainfrom
mostafasadeghidev:feat/media-multi-select-actions

Conversation

@mostafasadeghidev

Copy link
Copy Markdown
Contributor

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. 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 SiteExplorerPanel does. Media derives its floating windows from the selection during render — viewer at <= 1, bulk edit at >= 2 (MediaPage.tsx) — 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, so no server work is 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 delivers.

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.

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

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

mostafasadeghidev and others added 2 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>
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.

1 participant