You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Problem. Every backdrop/panel modal in this app closes on click without checking where the corresponding mousedown originated:
src/ui/results.js:362,850 — .cd-backdrop (onclick: close) / .cd-panel (onclick: (e) => e.stopPropagation()), used by both openRowsViewer and openCellDetail.
src/ui/detached-view.js:90 (backdrop) / :32 (panel) — the same shape for the graph overlay fallback.
src/ui/file-menu.js:238 (fm-dialog-backdrop) / :232 (fm-dialog-card) — the same shape for file-menu dialogs.
src/ui/shortcuts.js:47 (modal-backdrop) / :40 (modal-card) — same shape for the keyboard-shortcuts modal (missing from the original scan; added on review).
A browser's click event fires on the nearest common ancestor of the mousedown and mouseup targets — not the mousedown target. So any gesture that starts inside the panel/card but ends (mouseup) outside it — e.g. dragging a text selection in .cd-pre past the drawer's right edge before releasing — produces a click whose target is the backdrop itself. The panel's own stopPropagation() never runs (the panel isn't in that click's path at all), and the drawer/dialog closes unexpectedly.
Repro (today, on main, no code change): open a cell-detail drawer with enough text to select, click-drag a text selection from inside .cd-pre past the drawer's left edge into the backdrop, release the mouse there → drawer closes, discarding the in-progress selection.
Readiness notes (added on review — this issue is not yet implementation-ready without the following):
Intended behavior (needs to be this precisely, not just "check the origin"): a backdrop should close only when the mousedown that started the current gesture also landed on the backdrop itself (i.e., outside the panel/card) — not merely "the click target is the backdrop," since that's already true for the drag-out-of-panel case this issue reports. Concretely: track the element mousedown landed on; on the subsequent click, only call close() if that mousedown target was the backdrop (or anything outside the panel), matching the panel's own stopPropagation() intent from the other direction.
Required implementation shape — one shared helper, not four patches. This is not a judgment call: CLAUDE.md rule 5 requires extracting a shared primitive once a second consumer of a pattern appears, and this pattern already has four identical consumers (results.js ×2, detached-view.js, file-menu.js, shortcuts.js) all sharing the exact onclick: close / onclick: stopPropagation shape — well past that bar. Four independent per-call-site patches (mirroring attachDrawerResize's one-off swallow-listener) would be the same duplication this issue exists to remove, just moved one level down.
Add one small helper — e.g. attachBackdropClose(doc, backdrop, close) in src/ui/dom.js, alongside the other shared DOM primitives already there (h, s, withDocument, zoomScale, fixedAnchor) — that tracks whether mousedown landed on the backdrop itself and only calls close() on the following click if so. All four call sites switch their backdrop's onclick: close to wire through this helper instead (the panel's own stopPropagation() can likely be dropped once mousedown-tracking is in place, since a panel-only gesture would never satisfy "mousedown landed on the backdrop" anyway — worth confirming during implementation, not assumed here). attachDrawerResize's existing one-shot swallow listener in results.js should be reconciled with (or removed in favor of) this helper rather than left as a second, differently-shaped mechanism doing overlapping work.
Required tests (all four surfaces, not just cell-detail):
.cd-backdrop (both openRowsViewer and openCellDetail): a mousedown inside .cd-panel/.cd-pre followed by mouseup/click on the backdrop does not close it (the general case of the existing Cell-detail right-hand drawer: add horizontal resize #101-specific resize test — currently unverified for plain text selection).
A real backdrop click (mousedown + click both on the backdrop, nothing in between) still closes it, for all four surfaces.
The graph overlay (detached-view.js's openAsOverlay) and the file-menu confirm dialog (file-menu.js's openConfirm) get the same protection — not just the cell-detail drawer.
shortcuts.js's modal is in scope too (added above) — same protection, same test shape.
Escape and the explicit close buttons (.cd-close, .graph-overlay-close, fm-dialog-cancel, .close-btn) still close unconditionally, regardless of any pending mousedown tracking — this must not regress.
Problem. Every backdrop/panel modal in this app closes on
clickwithout checking where the correspondingmousedownoriginated:src/ui/results.js:362,850—.cd-backdrop(onclick: close) /.cd-panel(onclick: (e) => e.stopPropagation()), used by bothopenRowsViewerandopenCellDetail.src/ui/detached-view.js:90(backdrop) /:32(panel) — the same shape for the graph overlay fallback.src/ui/file-menu.js:238(fm-dialog-backdrop) /:232(fm-dialog-card) — the same shape for file-menu dialogs.src/ui/shortcuts.js:47(modal-backdrop) /:40(modal-card) — same shape for the keyboard-shortcuts modal (missing from the original scan; added on review).A browser's
clickevent fires on the nearest common ancestor of themousedownandmouseuptargets — not themousedowntarget. So any gesture that starts inside the panel/card but ends (mouseup) outside it — e.g. dragging a text selection in.cd-prepast the drawer's right edge before releasing — produces aclickwhose target is the backdrop itself. The panel's ownstopPropagation()never runs (the panel isn't in that click's path at all), and the drawer/dialog closes unexpectedly.Repro (today, on
main, no code change): open a cell-detail drawer with enough text to select, click-drag a text selection from inside.cd-prepast the drawer's left edge into the backdrop, release the mouse there → drawer closes, discarding the in-progress selection.Readiness notes (added on review — this issue is not yet implementation-ready without the following):
Intended behavior (needs to be this precisely, not just "check the origin"): a backdrop should close only when the
mousedownthat started the current gesture also landed on the backdrop itself (i.e., outside the panel/card) — not merely "the click target is the backdrop," since that's already true for the drag-out-of-panel case this issue reports. Concretely: track the elementmousedownlanded on; on the subsequentclick, only callclose()if thatmousedowntarget was the backdrop (or anything outside the panel), matching the panel's ownstopPropagation()intent from the other direction.Required implementation shape — one shared helper, not four patches. This is not a judgment call:
CLAUDE.mdrule 5 requires extracting a shared primitive once a second consumer of a pattern appears, and this pattern already has four identical consumers (results.js×2,detached-view.js,file-menu.js,shortcuts.js) all sharing the exactonclick: close/onclick: stopPropagationshape — well past that bar. Four independent per-call-site patches (mirroringattachDrawerResize's one-off swallow-listener) would be the same duplication this issue exists to remove, just moved one level down.Add one small helper — e.g.
attachBackdropClose(doc, backdrop, close)insrc/ui/dom.js, alongside the other shared DOM primitives already there (h,s,withDocument,zoomScale,fixedAnchor) — that tracks whethermousedownlanded on the backdrop itself and only callsclose()on the followingclickif so. All four call sites switch their backdrop'sonclick: closeto wire through this helper instead (the panel's ownstopPropagation()can likely be dropped once mousedown-tracking is in place, since a panel-only gesture would never satisfy "mousedown landed on the backdrop" anyway — worth confirming during implementation, not assumed here).attachDrawerResize's existing one-shot swallow listener inresults.jsshould be reconciled with (or removed in favor of) this helper rather than left as a second, differently-shaped mechanism doing overlapping work.Required tests (all four surfaces, not just cell-detail):
.cd-backdrop(bothopenRowsViewerandopenCellDetail): amousedowninside.cd-panel/.cd-prefollowed bymouseup/clickon the backdrop does not close it (the general case of the existing Cell-detail right-hand drawer: add horizontal resize #101-specific resize test — currently unverified for plain text selection).mousedown+clickboth on the backdrop, nothing in between) still closes it, for all four surfaces.detached-view.js'sopenAsOverlay) and the file-menu confirm dialog (file-menu.js'sopenConfirm) get the same protection — not just the cell-detail drawer.shortcuts.js's modal is in scope too (added above) — same protection, same test shape..cd-close,.graph-overlay-close,fm-dialog-cancel,.close-btn) still close unconditionally, regardless of any pendingmousedowntracking — this must not regress.