fix: backdrops close on any click without checking gesture origin#117
Merged
Conversation
A browser's click fires on the nearest common ancestor of mousedown/mouseup, not the mousedown target, so dragging a text selection from inside a drawer/panel past its edge before releasing produced a click targeting the backdrop directly — the panel's own stopPropagation() never ran and the modal closed, discarding the in-progress selection. Add a shared attachBackdropClose(backdrop, close) (src/ui/dom.js) that tracks where mousedown actually landed and gates close() on that, instead of pairing each backdrop's onclick:close with a panel onclick:stopPropagation. Wire it into all five affected surfaces: the cell-detail drawer, the rows-viewer pane, the graph overlay, the file-menu confirm dialog, and the keyboard-shortcuts modal. The cell-detail drawer's resize-drag one-shot click-swallow listener (#101) is superseded by the same general fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…sture-origin-110 # Conflicts: # CHANGELOG.md
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.
What & why
Closes #110.
A browser's
clickfires on the nearest common ancestor ofmousedown/mouseup, not themousedowntarget. So any gesture that starts inside a modal's panel/card — e.g. dragging a text selection — but ends (mouseup) outside it produced aclickwhose target was the backdrop itself. The panel's ownstopPropagation()never ran (the panel isn't in that click's path at all), so the drawer/dialog closed unexpectedly, discarding the in-progress selection.Adds a shared
attachBackdropClose(backdrop, close)(src/ui/dom.js) that tracks wheremousedownactually landed (capturing on the backdrop itself) and only callsclose()on the followingclickif that mousedown also landed on the backdrop — not merely wherever the click's target ends up. All five affected surfaces now share it instead of each pairing anonclick: closebackdrop with anonclick: stopPropagationpanel:results.js— the cell-detail drawer (openCellDetail) and the rows-viewer pane (openRowsViewer)detached-view.js— the graph overlay (openAsOverlay, shared by the schema graph / EXPLAIN pipeline / Data Pane expand)file-menu.js— the New-Library/Replace confirm dialogshortcuts.js— the keyboard-shortcuts modalThe cell-detail drawer's resize-drag one-shot click-swallow listener (#101) is superseded by this same general fix and removed.
Verification
Beyond the unit/e2e suites, manually verified live in a real Chromium browser via a throwaway Playwright harness driving real mouse events (mousedown → drag → mouseup → click): the drag-out-of-panel gesture no longer closes the drawer/modal, a genuine backdrop click still closes it, and Escape/✕ still close it. Confirmed the same harness fails against the pre-fix code, so the regression and the fix are both real.
Checklist
npm testpasses (the per-file coverage gate is non-negotiable)npm run buildsucceeds (single-filedist/sql.html)src/core/, network insrc/net/(injected fetch), DOM insrc/ui/CHANGELOG.md([Unreleased]) updatedinboxbugfix, not referenced by roadmap Roadmap to 1.0.0 #68🤖 Generated with Claude Code