feat(media): let the multi-selection reach the actions that imply it - #500
Open
mostafasadeghidev wants to merge 2 commits into
Open
feat(media): let the multi-selection reach the actions that imply it#500mostafasadeghidev wants to merge 2 commits into
mostafasadeghidev wants to merge 2 commits into
Conversation
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>
This was referenced Sep 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
openContextMenustored only the clicked asset, and the delete handler acted oncontextMenu.assetalone. 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
SiteExplorerPaneldoes. 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.
runPurgeAllsits 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:purgeAsset400s 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.
useTopmostEscapecarries 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.FloatingWindowuses 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