diff --git a/CHANGELOG.md b/CHANGELOG.md index cb8cb77..2ee48cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -230,6 +230,21 @@ auto-generated per-PR notes; this file is the curated, human-readable history. deliberately null/minimal-`app`-tolerant (`detached-view.js`, `explain-graph.js`, `schema-detail.js`, and `shortcuts.js` — which has a dedicated `delete app.document` test) were left untouched. (#106) +- Every backdrop/panel modal (the cell-detail drawer, the rows-viewer pane, the + graph overlay, the file-menu confirm dialog, the keyboard-shortcuts modal) + closed on **any** `click` reaching its backdrop, without checking where the + gesture's `mousedown` actually landed. A browser's `click` fires on the + nearest common ancestor of `mousedown`/`mouseup`, not the `mousedown` target, + so dragging a text selection from inside the panel past its edge before + releasing produced a `click` targeting the backdrop directly — the panel's + own `stopPropagation()` never ran (the panel wasn't in that click's + propagation path at all) and the modal closed, discarding the in-progress + selection. A new shared `attachBackdropClose` (`src/ui/dom.js`) tracks where + `mousedown` landed and only closes on a `click` whose `mousedown` also + landed on the backdrop itself; all five call sites now share it instead of + each pairing an `onclick: close` backdrop with an `onclick: stopPropagation` + panel. The cell-detail drawer's resize-drag one-shot click-swallow listener + (#101) is superseded by the same general fix. (#110) ## [0.1.5] - 2026-06-29 diff --git a/src/ui/detached-view.js b/src/ui/detached-view.js index 02bb900..211795e 100644 --- a/src/ui/detached-view.js +++ b/src/ui/detached-view.js @@ -7,7 +7,7 @@ // content-mount's CSS class ('graph-overlay-canvas' vs 'data-pane-body') so // each caller's own CSS still applies. -import { h, withDocument } from './dom.js'; +import { h, withDocument, attachBackdropClose } from './dom.js'; import { Icon } from './icons.js'; // Copy the theme/density data-attributes onto the child tab's so its @@ -29,7 +29,7 @@ function mirrorTheme(src, dst) { function buildPanel(mode, title) { const bar = h('div', { class: 'graph-overlay-bar' }, h('span', { class: 'graph-overlay-title' }, title)); const body = h('div', { class: mode === 'grid' ? 'data-pane-body' : 'graph-overlay-canvas', tabindex: '-1' }); - const panel = h('div', { class: 'graph-overlay-panel', onclick: (e) => e.stopPropagation() }, bar, body); + const panel = h('div', { class: 'graph-overlay-panel' }, bar, body); return { panel, bar, body }; } @@ -80,14 +80,17 @@ function openAsOverlay(app, mainDoc, title, mode, mount, onClose) { let teardown = null; let closed = false; let backdrop; + let detachBackdrop; const close = () => { if (closed) return; closed = true; + detachBackdrop(); backdrop.remove(); if (teardown) teardown(); onClose(); }; - backdrop = h('div', { class: 'graph-overlay', onclick: close }, panel); + backdrop = h('div', { class: 'graph-overlay' }, panel); + detachBackdrop = attachBackdropClose(backdrop, close); const closeBtn = h('button', { class: 'graph-overlay-close', title: 'Close (Esc)', onclick: close }, Icon.close()); const ret = mount({ doc: mainDoc, bar, body, close, closeBtn }); if (typeof ret === 'function') teardown = ret; diff --git a/src/ui/dom.js b/src/ui/dom.js index 1053c53..3ca73d9 100644 --- a/src/ui/dom.js +++ b/src/ui/dom.js @@ -85,3 +85,33 @@ export function fixedAnchor(rect, scale, opts = {}) { ? { top, right: Math.max(min, (opts.viewportW - rect.right) / scale) } : { top, left: Math.max(min, rect.left / scale) }; } + +// Wire a modal backdrop's close-on-click without the false positive from a +// gesture that starts inside the panel/card and ends over the backdrop (#110) +// — e.g. dragging a text selection past the panel's edge before releasing. A +// browser's `click` fires on the nearest common ancestor of the `mousedown` +// and `mouseup` targets, not the `mousedown` target, so that drag's `click` +// targets the backdrop directly even though the panel was never in its +// propagation path (the panel's own stopPropagation, if any, never runs). +// Track where `mousedown` actually landed instead: `close()` only fires when +// that mousedown's target was the backdrop itself, i.e. outside the panel. +// The mousedown listener is capturing on `backdrop` itself (not bubbling): +// capture visits `backdrop` on the way down to the real target, before any +// descendant's own stopPropagation can run, so an intervening stopPropagation +// inside the panel still can't hide the real mousedown target. +// Returns `detach()` — callers must invoke it from their own close(). +export function attachBackdropClose(backdrop, close) { + let downOnBackdrop = false; + const onDown = (e) => { downOnBackdrop = e.target === backdrop; }; + const onClick = () => { + const shouldClose = downOnBackdrop; + downOnBackdrop = false; // consume — a later click with no mousedown must not reuse it + if (shouldClose) close(); + }; + backdrop.addEventListener('mousedown', onDown, true); + backdrop.addEventListener('click', onClick); + return () => { + backdrop.removeEventListener('mousedown', onDown, true); + backdrop.removeEventListener('click', onClick); + }; +} diff --git a/src/ui/file-menu.js b/src/ui/file-menu.js index 9263e6a..c5228fb 100644 --- a/src/ui/file-menu.js +++ b/src/ui/file-menu.js @@ -5,7 +5,7 @@ // effect goes through an injected seam (app.saveJSON / app.saveStr / // app.downloadFile / app.FileReader / app.document), so it is fully testable. -import { h, zoomScale, fixedAnchor } from './dom.js'; +import { h, zoomScale, fixedAnchor, attachBackdropClose } from './dom.js'; import { Icon } from './icons.js'; import { flashToast } from './toast.js'; import { renderSavedHistory } from './saved-history.js'; @@ -228,16 +228,18 @@ function openConfirm(app, { title, body, confirmLabel, onConfirm }) { const doc = app.document; const close = () => { doc.removeEventListener('keydown', onKey, true); + detachBackdrop(); if (app.dom.fileDialog) { app.dom.fileDialog.remove(); app.dom.fileDialog = null; } }; const onKey = (e) => { if (e.key === 'Escape') { e.preventDefault(); close(); } }; - const card = h('div', { class: 'fm-dialog-card', onclick: (e) => e.stopPropagation() }, + const card = h('div', { class: 'fm-dialog-card' }, h('div', { class: 'fm-dialog-title' }, title), h('div', { class: 'fm-dialog-body' }, body), h('div', { class: 'fm-dialog-actions' }, h('button', { class: 'fm-dialog-cancel', onclick: close }, 'Cancel'), h('button', { class: 'fm-dialog-confirm', onclick: () => { close(); onConfirm(); } }, confirmLabel))); - const backdrop = h('div', { class: 'fm-dialog-backdrop', onclick: close }, card); + const backdrop = h('div', { class: 'fm-dialog-backdrop' }, card); + const detachBackdrop = attachBackdropClose(backdrop, close); app.dom.fileDialog = backdrop; doc.body.appendChild(backdrop); doc.addEventListener('keydown', onKey, true); diff --git a/src/ui/results.js b/src/ui/results.js index abdd4e8..591ea1e 100644 --- a/src/ui/results.js +++ b/src/ui/results.js @@ -2,7 +2,7 @@ // view for TSV/JSON output) plus the renderers. Heavy logic (sorting, axis // selection) lives in core/ and is reused here. -import { h, zoomScale, withDocument } from './dom.js'; +import { h, zoomScale, withDocument, attachBackdropClose } from './dom.js'; import { Icon } from './icons.js'; import { loadingPlaceholder } from './placeholder.js'; import { formatRows, formatBytes, isNumericType } from '../core/format.js'; @@ -334,9 +334,11 @@ export function openRowsViewer(app, entry) { const doc = app.document; let backdrop; let cancelDrawerDrag = () => {}; + let detachBackdrop; const onKey = (ev) => { if (ev.key === 'Escape' && isTopDrawer(doc, backdrop)) close(); }; function close() { cancelDrawerDrag(); + detachBackdrop(); if (backdrop) backdrop.remove(); doc.removeEventListener('keydown', onKey, true); } @@ -359,9 +361,10 @@ export function openRowsViewer(app, entry) { onCell: (name, type, value) => openCellDetail(app, name, type, value), })); paint(); - const panel = h('div', { class: 'cd-panel', onclick: (ev) => ev.stopPropagation() }, head, body); + const panel = h('div', { class: 'cd-panel' }, head, body); cancelDrawerDrag = attachDrawerResize(app, panel, doc); - backdrop = h('div', { class: 'cd-backdrop', onclick: close }, panel); + backdrop = h('div', { class: 'cd-backdrop' }, panel); + detachBackdrop = attachBackdropClose(backdrop, close); doc.body.appendChild(backdrop); doc.addEventListener('keydown', onKey, true); return backdrop; @@ -766,22 +769,17 @@ function isTopDrawer(doc, el) { * initial width from the persisted `cellDrawerPx` pref, clamped to the current * viewport, and appends the handle to `panel`. * - * Finishing a resize drag can end with the mouse over `.cd-backdrop` (dragging - * left grows the backdrop area under the cursor) — the `click` that follows - * mouseup then targets the backdrop directly (the nearest common ancestor of - * the mousedown/mouseup targets), bypassing `.cd-panel`'s own stopPropagation - * entirely and closing the drawer. A one-shot capturing `click` listener, - * installed at drag-start and removed after consuming exactly one event, - * swallows exactly that click. + * A resize drag that ends with the mouse over `.cd-backdrop` no longer needs a + * dedicated swallow-listener here: `attachBackdropClose` (#110) tracks where + * `mousedown` actually landed, and this handle is a `.cd-panel` descendant, so + * that drag's trailing click — wherever it targets — never closes the drawer. * * Returns `cancelDrag()`: the drawer's own `close()` (Escape / backdrop click / - * ✕) can fire while the mouse button is still down mid-drag, before that - * trailing click ever arrives — without this, the abandoned drag's `mousemove`/ - * `mouseup`/click-swallow listeners would linger on `win`/`doc` after the panel - * is gone, so a later unrelated mouseup would still persist a stale - * `cellDrawerPx` and a later unrelated click would be silently swallowed. - * `close()` must call it before removing the backdrop. A no-op if no drag is - * in progress. + * ✕) can fire while the mouse button is still down mid-drag — without this, + * the abandoned drag's `mousemove`/`mouseup` listeners would linger on `win` + * after the panel is gone, so a later unrelated mouseup would still persist a + * stale `cellDrawerPx`. `close()` must call it before removing the backdrop. + * A no-op if no drag is in progress. */ function attachDrawerResize(app, panel, doc) { // doc.defaultView is null for a detached document not yet attached to a real @@ -796,9 +794,6 @@ function attachDrawerResize(app, panel, doc) { title: 'Drag to resize', onmousedown: (ev) => { const startPx = app.state.cellDrawerPx; - const cleanup = () => { doc.removeEventListener('click', swallowClick, true); cancelActive = null; }; - const swallowClick = (e) => { e.stopPropagation(); cleanup(); }; - doc.addEventListener('click', swallowClick, true); const stopDrag = startDrag(ev, 'drawer', { win, state: app.state, @@ -807,7 +802,7 @@ function attachDrawerResize(app, panel, doc) { apply: (_axis, value) => { panel.style.width = value + 'px'; }, save: (name, value) => app.savePref(name, value), }); - cancelActive = () => { stopDrag(); app.state.cellDrawerPx = startPx; cleanup(); }; + cancelActive = () => { stopDrag(); app.state.cellDrawerPx = startPx; cancelActive = null; }; }, }); panel.appendChild(handle); @@ -819,9 +814,11 @@ export function openCellDetail(app, name, type, value, targetDoc) { const text = value == null ? '' : String(value); let backdrop; let cancelDrawerDrag = () => {}; + let detachBackdrop; const onKey = (e) => { if (e.key === 'Escape' && isTopDrawer(doc, backdrop)) close(); }; function close() { cancelDrawerDrag(); + detachBackdrop(); if (backdrop) backdrop.remove(); doc.removeEventListener('keydown', onKey, true); } @@ -841,7 +838,7 @@ export function openCellDetail(app, name, type, value, targetDoc) { type ? h('span', { class: 'cd-type' }, type) : null), h('button', { class: 'cd-close', title: 'Close (Esc)', onclick: close }, Icon.close())); - const panel = h('div', { class: 'cd-panel', onclick: (e) => e.stopPropagation() }, head); + const panel = h('div', { class: 'cd-panel' }, head); cancelDrawerDrag = attachDrawerResize(app, panel, doc); if (looksLikeHtml(text)) { @@ -865,7 +862,8 @@ export function openCellDetail(app, name, type, value, targetDoc) { showSource(); } - backdrop = h('div', { class: 'cd-backdrop', onclick: close }, panel); + backdrop = h('div', { class: 'cd-backdrop' }, panel); + detachBackdrop = attachBackdropClose(backdrop, close); doc.body.appendChild(backdrop); doc.addEventListener('keydown', onKey, true); return backdrop; diff --git a/src/ui/shortcuts.js b/src/ui/shortcuts.js index 25e6080..9ea984e 100644 --- a/src/ui/shortcuts.js +++ b/src/ui/shortcuts.js @@ -1,6 +1,6 @@ // Keyboard-shortcuts modal + the global key handler. -import { h } from './dom.js'; +import { h, attachBackdropClose } from './dom.js'; const SHORTCUTS = [ ['Run query', '⌘↵'], @@ -28,6 +28,7 @@ export function openShortcuts(app) { app.state.shortcutsOpen.value = true; const close = () => { app.state.shortcutsOpen.value = false; + detachBackdrop(); backdrop.remove(); doc.removeEventListener('keydown', escHandler); }; @@ -37,14 +38,15 @@ export function openShortcuts(app) { doc.addEventListener('keydown', escHandler); const rowOf = ([label, key]) => h('div', { class: 'row' }, h('span', { class: 'label' }, label), h('kbd', null, key)); - const card = h('div', { class: 'modal-card', onclick: (e) => e.stopPropagation() }, + const card = h('div', { class: 'modal-card' }, h('h2', null, 'Keyboard shortcuts'), ...SHORTCUTS.map(rowOf), h('div', { class: 'section-label' }, 'Schema tree — database · table · column'), ...GESTURES.map(rowOf), h('div', { class: 'close-row' }, h('button', { class: 'close-btn', onclick: close }, 'Close')), ); - const backdrop = h('div', { class: 'modal-backdrop', onclick: close }, card); + const backdrop = h('div', { class: 'modal-backdrop' }, card); + const detachBackdrop = attachBackdropClose(backdrop, close); doc.body.appendChild(backdrop); return { backdrop, close }; } diff --git a/tests/unit/detached-view.test.js b/tests/unit/detached-view.test.js index 462a90c..93e5263 100644 --- a/tests/unit/detached-view.test.js +++ b/tests/unit/detached-view.test.js @@ -178,11 +178,24 @@ describe('openInDetachedTab — overlay fallback', () => { const app = { document, openWindow: () => null, state: detachedState() }; openInDetachedTab(app, { title: 'X', mode: 'graph', mount: () => {} }); expect(app.state.detachedView.value).toBe(1); - document.querySelector('.graph-overlay').dispatchEvent(new Event('click', { bubbles: true })); + const overlay = document.querySelector('.graph-overlay'); + overlay.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + overlay.dispatchEvent(new Event('click', { bubbles: true })); expect(document.querySelector('.graph-overlay')).toBeNull(); expect(app.state.detachedView.value).toBe(0); }); + it('a gesture starting inside the panel and ending on the backdrop does not close it (#110)', () => { + openInDetachedTab({ document, openWindow: () => null, state: detachedState() }, { + title: 'X', mode: 'graph', mount: ({ body }) => { body.textContent = 'selectable content'; }, + }); + const overlay = document.querySelector('.graph-overlay'); + overlay.querySelector('.graph-overlay-canvas').dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + // The browser's post-drag click targets the backdrop directly. + overlay.dispatchEvent(new MouseEvent('click', { bubbles: true })); + expect(document.body.contains(overlay)).toBe(true); + }); + it('runs the teardown fn exactly once even when close() is called twice directly', () => { const teardown = vi.fn(); let captured; diff --git a/tests/unit/dom.test.js b/tests/unit/dom.test.js index b7211c6..3a6d15c 100644 --- a/tests/unit/dom.test.js +++ b/tests/unit/dom.test.js @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; -import { h, s, withDocument, zoomScale, fixedAnchor } from '../../src/ui/dom.js'; +import { h, s, withDocument, zoomScale, fixedAnchor, attachBackdropClose } from '../../src/ui/dom.js'; const SVG_NS = 'http://www.w3.org/2000/svg'; @@ -93,6 +93,67 @@ describe('s (SVG namespace)', () => { }); }); +describe('attachBackdropClose (#110)', () => { + function setup() { + const panel = h('div', { class: 'panel' }, h('div', { class: 'inner' }, 'x')); + const backdrop = h('div', { class: 'backdrop' }, panel); + document.body.appendChild(backdrop); + const close = vi.fn(); + const detach = attachBackdropClose(backdrop, close); + return { backdrop, panel, close, detach }; + } + const down = (el) => el.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + const click = (el) => el.dispatchEvent(new MouseEvent('click', { bubbles: true })); + + it('closes when mousedown and click both land on the backdrop itself', () => { + const { backdrop, close } = setup(); + down(backdrop); + click(backdrop); + expect(close).toHaveBeenCalledTimes(1); + backdrop.remove(); + }); + it('does not close when mousedown lands inside the panel, even if the click targets the backdrop', () => { + // Simulates a drag that starts inside the panel and ends outside it — the + // browser's click then targets the backdrop directly (#110's repro). + const { backdrop, panel, close } = setup(); + down(panel.querySelector('.inner')); + click(backdrop); + expect(close).not.toHaveBeenCalled(); + backdrop.remove(); + }); + it('does not close on a click with no preceding mousedown', () => { + const { backdrop, close } = setup(); + click(backdrop); + expect(close).not.toHaveBeenCalled(); + backdrop.remove(); + }); + it('does not close a normal click inside the panel (mousedown and click both on a panel descendant)', () => { + const { panel, close, backdrop } = setup(); + const inner = panel.querySelector('.inner'); + down(inner); + click(inner); // bubbles to the backdrop; not stopped by the panel + expect(close).not.toHaveBeenCalled(); + backdrop.remove(); + }); + it('does not reuse a stale mousedown-on-backdrop flag for a later click with no mousedown of its own', () => { + const { backdrop, close } = setup(); + down(backdrop); + click(backdrop); + expect(close).toHaveBeenCalledTimes(1); + click(backdrop); // no mousedown before this one — must not close again + expect(close).toHaveBeenCalledTimes(1); + backdrop.remove(); + }); + it('detach() removes both listeners — a later mousedown+click on the backdrop no longer closes', () => { + const { backdrop, close, detach } = setup(); + detach(); + down(backdrop); + click(backdrop); + expect(close).not.toHaveBeenCalled(); + backdrop.remove(); + }); +}); + describe('h', () => { it('builds an element with no props', () => { const el = h('div'); diff --git a/tests/unit/explain-graph.test.js b/tests/unit/explain-graph.test.js index cd7271b..ca3772b 100644 --- a/tests/unit/explain-graph.test.js +++ b/tests/unit/explain-graph.test.js @@ -164,6 +164,7 @@ describe('openPipelineFullscreen', () => { // backdrop click closes openPipelineFullscreen(APP, DOT); overlay = overlayOf(); + overlay.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); overlay.dispatchEvent(new Event('click', { bubbles: true })); expect(document.body.contains(overlay)).toBe(false); }); @@ -365,7 +366,8 @@ describe('schema lineage graph', () => { expect(document.body.contains(overlay)).toBe(false); openSchemaView(overlayApp({ openNodeDetail: vi.fn() })).render(GRAPH); overlay = overlayOf(); - overlay.dispatchEvent(new Event('click', { bubbles: true })); // backdrop + overlay.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); // backdrop + overlay.dispatchEvent(new Event('click', { bubbles: true })); expect(document.body.contains(overlay)).toBe(false); }); diff --git a/tests/unit/file-menu.test.js b/tests/unit/file-menu.test.js index 45fa993..b6f4d13 100644 --- a/tests/unit/file-menu.test.js +++ b/tests/unit/file-menu.test.js @@ -281,7 +281,9 @@ describe('New Library + confirm dialogs', () => { expect(app.state.savedQueries).toHaveLength(2); // backdrop click openNew(); - click(document.querySelector('.fm-dialog-backdrop')); + const backdrop = document.querySelector('.fm-dialog-backdrop'); + backdrop.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + click(backdrop); expect(document.querySelector('.fm-dialog-backdrop')).toBeNull(); // card click keeps it open; Escape closes it openNew(); @@ -291,4 +293,16 @@ describe('New Library + confirm dialogs', () => { expect(document.querySelector('.fm-dialog-backdrop')).toBeNull(); expect(app.state.savedQueries).toHaveLength(2); }); + + it('a gesture starting on the card and ending on the backdrop does not dismiss it (#110)', () => { + const app = mount(); + app.state.savedQueries = [{ id: 's1', name: 'A', sql: '1', favorite: false }]; + openFileMenu(app); + click(item(/New Library/)); + const backdrop = document.querySelector('.fm-dialog-backdrop'); + const card = document.querySelector('.fm-dialog-card'); + card.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + backdrop.dispatchEvent(new Event('click', { bubbles: true })); // click's target is the backdrop + expect(document.querySelector('.fm-dialog-backdrop')).not.toBeNull(); + }); }); diff --git a/tests/unit/results.test.js b/tests/unit/results.test.js index e95b697..64ec213 100644 --- a/tests/unit/results.test.js +++ b/tests/unit/results.test.js @@ -5,6 +5,12 @@ import { newResult } from '../../src/core/stream.js'; import { schemaKey, chartRowCap } from '../../src/core/chart-data.js'; const click = (el) => el.dispatchEvent(new Event('click', { bubbles: true })); +// A genuine backdrop click: mousedown and click both land on `el` itself +// (#110's attachBackdropClose gates close() on where mousedown landed). +const backdropClick = (el) => { + el.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + el.dispatchEvent(new Event('click', { bubbles: true })); +}; function appWithResult(result, over = {}) { const app = makeApp(); @@ -374,13 +380,26 @@ describe('openCellDetail', () => { document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); expect(document.querySelector('.cd-backdrop')).toBeNull(); openCellDetail(app, 'c', 'String', 'x'); - click(document.querySelector('.cd-backdrop')); + backdropClick(document.querySelector('.cd-backdrop')); expect(document.querySelector('.cd-backdrop')).toBeNull(); openCellDetail(app, 'c', 'String', 'x'); - click(document.querySelector('.cd-panel')); // stopPropagation → stays open + backdropClick(document.querySelector('.cd-panel')); // mousedown+click inside the panel → stays open expect(document.querySelector('.cd-backdrop')).not.toBeNull(); document.querySelector('.cd-backdrop').remove(); }); + it('a gesture starting inside the panel and ending (mouseup/click) on the backdrop does not close it (#110)', () => { + const app = makeApp(); + openCellDetail(app, 'c', 'String', 'a selectable value'); + const backdrop = document.querySelector('.cd-backdrop'); + const pre = backdrop.querySelector('.cd-pre'); + pre.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); // drag starts inside the panel + // The click that follows targets the backdrop directly — the nearest + // common ancestor of the mousedown (inside .cd-pre) and mouseup targets. + backdrop.dispatchEvent(new MouseEvent('click', { bubbles: true })); + expect(document.querySelector('.cd-backdrop')).not.toBeNull(); + backdropClick(backdrop); // a later, genuine backdrop click still closes it + expect(document.querySelector('.cd-backdrop')).toBeNull(); + }); it('builds in a given targetDoc instead of the main document (detached-tab safe)', () => { const childDoc = document.implementation.createHTMLDocument(''); openCellDetail(makeApp(), 'c', 'String', 'x', childDoc); @@ -430,7 +449,6 @@ describe('cell-detail drawer resize (#101)', () => { window.dispatchEvent(new MouseEvent('mouseup', {})); expect(app.state.cellDrawerPx).toBe(524); expect(app.savePref).toHaveBeenCalledWith('cellDrawerPx', 524); - click(panel); // the browser's post-mouseup click — consumes the one-shot swallow listener document.querySelector('.cd-backdrop').remove(); }); it('clamps mid-drag width to [320, 92vw]', () => { @@ -444,7 +462,6 @@ describe('cell-detail drawer resize (#101)', () => { window.dispatchEvent(new MouseEvent('mousemove', { clientX: -2000 })); // way over → 92vw cap expect(panel.style.width).toBe(1024 * 0.92 + 'px'); window.dispatchEvent(new MouseEvent('mouseup', {})); - click(panel); // consumes the one-shot swallow listener document.querySelector('.cd-backdrop').remove(); }); it('finishing a resize drag with the mouse over the backdrop does not close the drawer; a later genuine click still does', () => { @@ -457,11 +474,13 @@ describe('cell-detail drawer resize (#101)', () => { window.dispatchEvent(new MouseEvent('mouseup', {})); // The browser follows a drag's mouseup with a `click` targeting the nearest // common ancestor of the mousedown/mouseup targets — here, since mouseup - // landed outside `.cd-panel`, that's the backdrop itself. + // landed outside `.cd-panel`, that's the backdrop itself. attachBackdropClose + // (#110) gates close() on the mousedown target (the handle, inside the + // panel), so this click alone does not close it. backdrop.dispatchEvent(new MouseEvent('click', { bubbles: true })); - expect(document.querySelector('.cd-backdrop')).not.toBeNull(); // swallowed — stays open - click(backdrop); // a later, unrelated click outside the drawer - expect(document.querySelector('.cd-backdrop')).toBeNull(); // closes normally + expect(document.querySelector('.cd-backdrop')).not.toBeNull(); // stays open + backdropClick(backdrop); // a later, genuine backdrop click still closes it + expect(document.querySelector('.cd-backdrop')).toBeNull(); }); it('closing the drawer mid-drag (Escape, mouse still down) cancels the drag: reverts the width, and does not leak listeners that swallow a later click or persist a stale width on a later mouseup', () => { const app = makeApp(); @@ -475,8 +494,8 @@ describe('cell-detail drawer resize (#101)', () => { expect(document.querySelector('.cd-backdrop')).toBeNull(); expect(app.state.cellDrawerPx).toBe(560); // reverted — the abandoned drag never committed - // The drag's own mousemove/mouseup listeners and the click-swallow listener - // must have been torn down by the cancel, not just left to resolve later. + // The drag's own mousemove/mouseup listeners must have been torn down by + // the cancel, not just left to resolve later. window.dispatchEvent(new MouseEvent('mousemove', { clientX: 100 })); window.dispatchEvent(new MouseEvent('mouseup', {})); expect(app.state.cellDrawerPx).toBe(560); // a stray mouseup doesn't resurrect + persist the drag @@ -484,7 +503,7 @@ describe('cell-detail drawer resize (#101)', () => { openCellDetail(app, 'c2', 'String', 'y'); // an unrelated, later click must work normally const backdrop2 = document.querySelector('.cd-backdrop'); - click(backdrop2); + backdropClick(backdrop2); expect(document.querySelector('.cd-backdrop')).toBeNull(); }); }); @@ -950,7 +969,7 @@ describe('EXPLAIN views', () => { click(expand); const overlay = document.body.querySelector('.graph-overlay'); expect(overlay).not.toBeNull(); - overlay.dispatchEvent(new Event('click', { bubbles: true })); // backdrop click closes + cleans up + backdropClick(overlay); // backdrop click closes + cleans up expect(document.body.querySelector('.graph-overlay')).toBeNull(); }); @@ -1067,7 +1086,7 @@ describe('multiquery script grid (#83)', () => { // reopen + close via backdrop click click(app.dom.resultsRegion.querySelector('.script-cell.rows')); backdrop = document.querySelector('.cd-backdrop'); - click(backdrop); + backdropClick(backdrop); expect(document.querySelector('.cd-backdrop')).toBeNull(); }); @@ -1094,7 +1113,6 @@ describe('multiquery script grid (#83)', () => { expect(panel.style.width).toBe('524px'); window.dispatchEvent(new MouseEvent('mouseup', {})); expect(app.state.cellDrawerPx).toBe(524); - click(panel); // consumes the one-shot swallow listener (see results.js attachDrawerResize) document.querySelector('.cd-backdrop').remove(); }); diff --git a/tests/unit/shortcuts.test.js b/tests/unit/shortcuts.test.js index 1f45512..cccdbc8 100644 --- a/tests/unit/shortcuts.test.js +++ b/tests/unit/shortcuts.test.js @@ -26,16 +26,28 @@ describe('openShortcuts', () => { it('closes when the backdrop is clicked', () => { const app = makeApp({ document }); openShortcuts(app); - document.querySelector('.modal-backdrop').dispatchEvent(new Event('click')); + const backdrop = document.querySelector('.modal-backdrop'); + backdrop.dispatchEvent(new MouseEvent('mousedown')); + backdrop.dispatchEvent(new Event('click')); expect(app.state.shortcutsOpen.value).toBe(false); }); - it('card click does not close (stopPropagation)', () => { + it('card click does not close', () => { const app = makeApp({ document }); openShortcuts(app); const card = document.querySelector('.modal-card'); + card.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); card.dispatchEvent(new Event('click', { bubbles: true })); expect(app.state.shortcutsOpen.value).toBe(true); }); + it('a gesture starting on the card and ending on the backdrop does not close it (#110)', () => { + const app = makeApp({ document }); + openShortcuts(app); + const backdrop = document.querySelector('.modal-backdrop'); + const card = document.querySelector('.modal-card'); + card.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + backdrop.dispatchEvent(new Event('click', { bubbles: true })); // click's target is the backdrop + expect(app.state.shortcutsOpen.value).toBe(true); + }); it('defaults document to global', () => { const app = makeApp(); delete app.document;