Repository navigation
Fix: context menu items fire nothing on click - #31
Merged
Merged
Conversation
The dismiss-on-outside-click listener was a capture-phase handler on
document, which runs before the event reaches (or bubbles back out of)
any descendant -- including the menu's own buttons. So it closed the
menu on every mousedown inside it too, unmounting the component before
the button's click ever fired. The container's onMouseDown{stopPropagation}
could not prevent this: stopPropagation only affects listeners still
pending in the dispatch order, and a document-level capture listener has
already run by the time a bubble-phase React handler further down the
tree executes.
Fixed by checking containment in the capture handler itself (only
dismiss when the mousedown target is outside the menu), rather than
relying on a bubble-phase stopPropagation to save it after the fact.
Reproduced with a real Puppeteer mouse-click sequence (not
element.click(), which bypasses the actual event order this bug lives
in) against a locally-run build: right-click a folder, click 'Share via
SMB' -- before the fix, no modal opens and no request fires; after,
both work. Verified the same for 'New folder' end-to-end including the
resulting POST, and confirmed clicking outside the menu still correctly
dismisses it (no regression). Backend's 51 tests unaffected, as
expected for a frontend-only fix.
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.
Fixes the reported bug: right-clicking a folder opens the menu, but every item -- Share via SMB, rename, delete, copy, etc. -- silently did nothing, no API request ever sent.
Root cause: the outside-click dismiss listener ran in the capture phase on
document, which fires before the event reaches (or bubbles back out of) the menu's own buttons -- so it closed the menu on every click inside it too, unmounting the component before the button's own click handler ran.stopPropagation()on the container couldn't save it, since that's a bubble-phase React handler and the capture-phase document listener had already run by then.Fixed by checking click containment in the capture handler itself, instead of relying on stopPropagation after the fact.
Verified with a real Puppeteer mouse-click sequence (not
element.click(), which bypasses the actual event ordering this bug lives in) against a local build: right-click a folder -> Share via SMB now opens the dialog; New folder now fires the real mkdir POST; clicking outside the menu still correctly dismisses it (no regression).