Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 6 additions & 3 deletions src/ui/detached-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <html> so its
Expand All @@ -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 };
}

Expand Down Expand Up @@ -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;
Expand Down
30 changes: 30 additions & 0 deletions src/ui/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
}
8 changes: 5 additions & 3 deletions src/ui/file-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
44 changes: 21 additions & 23 deletions src/ui/results.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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);
}
Expand All @@ -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)) {
Expand All @@ -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;
Expand Down
8 changes: 5 additions & 3 deletions src/ui/shortcuts.js
Original file line number Diff line number Diff line change
@@ -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', '⌘↵'],
Expand Down Expand Up @@ -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);
};
Expand All @@ -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 };
}
Expand Down
15 changes: 14 additions & 1 deletion tests/unit/detached-view.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
63 changes: 62 additions & 1 deletion tests/unit/dom.test.js
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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');
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/explain-graph.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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);
});

Expand Down
Loading