Skip to content

Commit cd1c6b7

Browse files
fix(schema): harden the new-tab lineage view + default to the day theme
Review follow-ups for the full schema view (PR #55): - Overlay fallback: guard render()/fail() once the view is closed so a late-arriving fetch can't attach (and leak) ⌘Z/⌘Y handlers on the main document that would swallow the editor's own undo/redo. - expandSchemaGraph: wrap the whole post-open flow in try/catch → view.fail, so a token-refresh rejection or build throw can't strand the tab on "Loading…". - Node ⌘-drag: end the drag on a buttons===0 move (release off-window no longer sticks the node to the cursor); re-read the viewBox each move so a ⌘/wheel zoom mid-drag keeps tracking the pointer. - Re-route edge LABELS with their edges on a move (not just the paths), and put a straightened 2-point edge's label on the segment midpoint instead of the target arrowhead. - Grow the layout bounds on a move so Fit/double-click can reframe a node dragged past dagre's original extent. - Refit the full view + overlays on window resize (scoped opt; inline panes unchanged) so drag/pan keep tracking after a resize. - Overlay theme toggle routes through app.toggleTheme (state + saved pref + header icon stay in sync); the tab toggle stays local. Icon rebuilt inside withDocument so it's created in the view's own realm. - Clear the latched .modkey cursor on window blur. - Reuse: panBy + the new-tab drag share dragDeltaToSvg; precompute each node's incident edges once instead of rescanning per mousemove frame. Default theme is now light (day) — state fallback + the template's initial paint, so there's no dark flash before JS runs. Tests added for every new branch; 994 pass, per-file coverage gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1pC6hkksFXLogTCVgNSgK
1 parent ef4b1de commit cd1c6b7

7 files changed

Lines changed: 262 additions & 58 deletions

File tree

build/template.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!DOCTYPE html>
2-
<html lang="en" data-theme="dark">
2+
<html lang="en" data-theme="light">
33
<head>
44
<meta charset="UTF-8">
55
<meta name="viewport" content="width=device-width, initial-scale=1">

src/state.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export function createState(read = { loadJSON, loadStr }) {
4343
const num = (key, dflt, lo, hi) => clamp(parseFloat(read.loadStr(key, String(dflt))), lo, hi);
4444
return {
4545
nextTabId: 2,
46-
theme: read.loadStr(KEYS.theme, 'dark'),
46+
theme: read.loadStr(KEYS.theme, 'light'),
4747
density: 'comfortable',
4848
sidebarPx: clamp(parseInt(read.loadStr(KEYS.sidebarPx, '248'), 10), 180, 420),
4949
editorPct: num(KEYS.editorPct, 45, 15, 85),

src/ui/app.js

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -563,34 +563,35 @@ export function createApp(env = {}) {
563563
// Open the view synchronously so a real tab survives the click gesture (a
564564
// pop-up opened after an await is blocked); fill it once the lineage loads.
565565
const view = openSchemaView(app);
566-
await ensureConfig();
567-
if (!(await getToken())) { chCtx.onSignedOut(); view.fail('Sign in to view the schema graph.'); return; }
568-
let lineage;
566+
// Everything after the synchronous open is wrapped: a token-refresh rejection,
567+
// a lineage/cards fetch failure, or a graph-build throw must surface in the view
568+
// (fail) instead of leaving the just-opened tab/overlay stranded on "Loading…".
569569
try {
570+
await ensureConfig();
571+
if (!(await getToken())) { chCtx.onSignedOut(); view.fail('Sign in to view the schema graph.'); return; }
570572
// Walk lineage transitively across DB boundaries (soft-capped) — pulls in
571573
// objects an other database references, instead of dead-ending at the edge.
572-
lineage = await ch.loadLineageTransitive(chCtx, focus);
574+
const lineage = await ch.loadLineageTransitive(chCtx, focus);
575+
const g = buildSchemaGraph(lineage.rows, focus);
576+
const ex = expandLineage(g, focus.db); // closure around focus.db, tags external nodes
577+
// Card metadata for every database the expansion reached (external nodes too).
578+
const dbs = [...new Set(ex.nodes.map((n) => n.db).filter(Boolean))];
579+
const cards = await ch.loadSchemaCards(chCtx, dbs);
580+
const cardGraph = buildCardGraph({ nodes: ex.nodes, edges: ex.edges },
581+
{ tables: lineage.rows.tables, columnsByKey: cards.columnsByKey, skipByKey: cards.skipByKey });
582+
// Persist manually-moved node positions per result: the map hangs off the live
583+
// schemaGraph result (captured above) so re-opening keeps the layout.
584+
const positions = (sg && sg.savedPositions) || {};
585+
if (sg) sg.savedPositions = positions;
586+
view.render({
587+
nodes: cardGraph.nodes, edges: cardGraph.edges, focus,
588+
tableCount: (lineage.rows.tables || []).length,
589+
truncated: lineage.truncated || ex.truncated,
590+
savedPositions: positions,
591+
});
573592
} catch {
574593
view.fail('Could not load the schema graph');
575-
return;
576594
}
577-
const g = buildSchemaGraph(lineage.rows, focus);
578-
const ex = expandLineage(g, focus.db); // closure around focus.db, tags external nodes
579-
// Card metadata for every database the expansion reached (external nodes too).
580-
const dbs = [...new Set(ex.nodes.map((n) => n.db).filter(Boolean))];
581-
const cards = await ch.loadSchemaCards(chCtx, dbs);
582-
const cardGraph = buildCardGraph({ nodes: ex.nodes, edges: ex.edges },
583-
{ tables: lineage.rows.tables, columnsByKey: cards.columnsByKey, skipByKey: cards.skipByKey });
584-
// Persist manually-moved node positions per result: the map hangs off the live
585-
// schemaGraph result (captured above) so re-opening keeps the layout.
586-
const positions = (sg && sg.savedPositions) || {};
587-
if (sg) sg.savedPositions = positions;
588-
view.render({
589-
nodes: cardGraph.nodes, edges: cardGraph.edges, focus,
590-
tableCount: (lineage.rows.tables || []).length,
591-
truncated: lineage.truncated || ex.truncated,
592-
savedPositions: positions,
593-
});
594595
}
595596

596597
// Open the detail pane for a clicked fullscreen node: lazily load the table's full
@@ -793,6 +794,9 @@ export function createApp(env = {}) {
793794
doc.documentElement.setAttribute('data-theme', app.state.theme);
794795
if (app.dom.themeBtn) app.dom.themeBtn.replaceChildren(app.state.theme === 'dark' ? Icon.sun() : Icon.moon());
795796
}
797+
// Exposed so the schema-view overlay can drive the same toggle (keeps state +
798+
// saved pref + header icon in sync rather than flipping data-theme behind them).
799+
app.toggleTheme = toggleTheme;
796800

797801
// --- actions registry --------------------------------------------------
798802
app.actions = {

src/ui/explain-graph.js

Lines changed: 82 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ function attachPanZoom(container, svg, dims, opts = {}) {
4747
// fitWidth: frame the graph to fill the container's WIDTH and let the height
4848
// overflow (pan/scroll down) — used by the schema full view, which can be tall.
4949
const fitWidth = !!opts.fitWidth;
50+
// refitOnResize: re-fit when the window resizes. Set for the standalone schema
51+
// tab + the fullscreen overlays (whose container tracks the viewport); left off
52+
// for the small inline result pane, which re-renders often and shouldn't reset
53+
// a user's pan/zoom on every layout change.
54+
const refitOnResize = !!opts.refitOnResize;
5055
svg.setAttribute('width', '100%');
5156
svg.setAttribute('height', '100%');
5257
svg.setAttribute('preserveAspectRatio', 'xMidYMid meet');
@@ -69,8 +74,8 @@ function attachPanZoom(container, svg, dims, opts = {}) {
6974
// Pan by pixel deltas (drag grabs the content; wheel scrolls the viewport — the
7075
// caller passes the appropriate sign).
7176
const panBy = (dxPx, dyPx) => {
72-
const r = container.getBoundingClientRect();
73-
vb = panBox(vb, dxPx * (vb.w / r.width), dyPx * (vb.h / r.height));
77+
const { dx, dy } = dragDeltaToSvg(dxPx, dyPx, vb, container.getBoundingClientRect());
78+
vb = panBox(vb, dx, dy);
7479
apply();
7580
};
7681
const centre = () => { const r = container.getBoundingClientRect(); return { x: r.left + r.width / 2, y: r.top + r.height / 2 }; };
@@ -95,6 +100,16 @@ function attachPanZoom(container, svg, dims, opts = {}) {
95100
container.addEventListener('mouseup', end);
96101
container.addEventListener('mouseleave', end);
97102
container.addEventListener('dblclick', fit);
103+
// Refit on window resize so the viewBox aspect keeps matching the container —
104+
// otherwise preserveAspectRatio letterboxes and drag/pan stop tracking the
105+
// pointer (notably when the standalone schema tab is resized). The listener
106+
// removes itself once the container leaves the DOM (overlay/tab closed); a
107+
// detached document (defaultView null) never gets one in the first place.
108+
const win = container.ownerDocument.defaultView;
109+
if (win && refitOnResize) {
110+
const onResize = () => { if (container.isConnected) fit(); else win.removeEventListener('resize', onResize); };
111+
win.addEventListener('resize', onResize);
112+
}
98113

99114
apply();
100115
return { fit, zoomIn: () => { const c = centre(); zoomAt(ZOOM_STEP, c.x, c.y); }, zoomOut: () => { const c = centre(); zoomAt(1 / ZOOM_STEP, c.x, c.y); } };
@@ -126,8 +141,14 @@ function graphSvgWithEdges(g, edgeClass, edgeLabel) {
126141
svg.appendChild(s('path', { class: edgeClass(e), d, 'marker-end': 'url(#eg-arrow)', 'data-eidx': i, 'data-from': e.from, 'data-to': e.to }));
127142
const lbl = edgeLabel && edgeLabel(e);
128143
if (lbl) {
129-
const mid = e.points[Math.floor(e.points.length / 2)];
130-
svg.appendChild(s('text', { class: 'eg-edge-label', x: mid.x, y: mid.y - 3, 'text-anchor': 'middle' }, lbl));
144+
// A straightened (2-point) edge has no real mid-vertex, so points[len/2]
145+
// would land on the target endpoint — use the segment midpoint instead.
146+
// data-lbl-eidx lets the move handler reposition the label with its edge.
147+
const pts = e.points;
148+
const mid = pts.length === 2
149+
? { x: (pts[0].x + pts[1].x) / 2, y: (pts[0].y + pts[1].y) / 2 }
150+
: pts[Math.floor(pts.length / 2)];
151+
svg.appendChild(s('text', { class: 'eg-edge-label', x: mid.x, y: mid.y - 3, 'text-anchor': 'middle', 'data-lbl-eidx': i }, lbl));
131152
}
132153
});
133154
return svg;
@@ -296,7 +317,7 @@ function openGraphFullscreen(app, title, build) {
296317
canvas.appendChild(placeholder('Nothing to display.'));
297318
} else {
298319
canvas.appendChild(built.svg);
299-
const pz = attachPanZoom(canvas, built.svg, built);
320+
const pz = attachPanZoom(canvas, built.svg, built, { refitOnResize: true });
300321
actions.appendChild(zoomControls(pz));
301322
}
302323
actions.appendChild(h('button', { class: 'graph-overlay-close', title: 'Close (Esc)', onclick: close }, Icon.close()));
@@ -373,21 +394,26 @@ function focusLabel(focus) {
373394

374395
// Day/night switcher for the view's own document — mirrors the main window's
375396
// toggle (sun while dark → click for light; moon while light → click for dark).
376-
function themeToggle(doc) {
397+
// `onToggle` is the app's real toggleTheme: passed only when the view IS the main
398+
// document (overlay fallback) so app.state/the saved pref/the header button stay
399+
// in sync; in a separate tab it's omitted and the flip is local + ephemeral. The
400+
// icon is rebuilt inside withDocument(doc) so it's created in the view's own realm.
401+
function themeToggle(doc, onToggle) {
377402
const icon = () => (doc.documentElement.getAttribute('data-theme') === 'light' ? Icon.moon() : Icon.sun());
378403
const btn = h('button', { class: 'res-act', title: 'Toggle theme' }, icon());
379404
btn.addEventListener('click', () => {
380-
doc.documentElement.setAttribute('data-theme', doc.documentElement.getAttribute('data-theme') === 'light' ? 'dark' : 'light');
381-
btn.replaceChildren(icon());
405+
if (onToggle) onToggle(); // overlay: app's toggle flips data-theme + state + pref + header icon
406+
else doc.documentElement.setAttribute('data-theme', doc.documentElement.getAttribute('data-theme') === 'light' ? 'dark' : 'light');
407+
withDocument(doc, () => btn.replaceChildren(icon()));
382408
});
383409
return btn;
384410
}
385411

386-
// Truncation banner text (null when the lineage wasn't soft-capped).
412+
// Truncation banner text (null when the lineage wasn't soft-capped). Only called
413+
// from render() with a populated graph (the nodeCount > 0 branch), so graph.nodes
414+
// is always present here.
387415
function schemaNote(graph) {
388-
return graph && graph.truncated
389-
? 'Lineage truncated — showing ' + (((graph.nodes && graph.nodes.length) || 0)) + ' objects'
390-
: null;
416+
return graph.truncated ? 'Lineage truncated — showing ' + graph.nodes.length + ' objects' : null;
391417
}
392418

393419
// ⌘/Ctrl drives a hand cursor (.modkey) and gates node dragging: a ⌘/Ctrl+drag
@@ -402,19 +428,33 @@ function attachSchemaInteractions(canvas, svg, built, targetDoc, positions, onCh
402428
svg.querySelectorAll('g.eg-card[data-node-id]').forEach((g) => cardById.set(g.getAttribute('data-node-id'), g));
403429
const pathByIdx = new Map();
404430
svg.querySelectorAll('path[data-eidx]').forEach((p) => pathByIdx.set(+p.getAttribute('data-eidx'), p));
431+
const labelByIdx = new Map();
432+
svg.querySelectorAll('text[data-lbl-eidx]').forEach((t) => labelByIdx.set(+t.getAttribute('data-lbl-eidx'), t));
433+
// Each node's incident-edge indices are fixed for the view's lifetime, so map
434+
// them once here rather than rescanning every edge on every drag-move frame.
435+
const incidentById = new Map();
436+
nodes.forEach((n) => incidentById.set(n.id, incidentEdges(edges, n.id)));
405437
const getVb = () => { const a = svg.getAttribute('viewBox').split(' ').map(Number); return { x: a[0], y: a[1], w: a[2], h: a[3] }; };
406438
const history = createMoveHistory();
407439

408440
// Move a node to an absolute position: translate its card, re-route only its
409-
// incident edges, and update the persisted map. Shared by live drag + undo/redo.
441+
// incident edges (and their labels), grow the layout bounds, and update the
442+
// persisted map. Shared by live drag + undo/redo.
410443
const placeAt = (id, x, y) => {
411444
const node = byId.get(id);
412445
node.x = x; node.y = y;
413446
cardById.get(id).setAttribute('transform', 'translate(' + (x - node.x0) + ' ' + (y - node.y0) + ')');
414-
for (const i of incidentEdges(edges, id)) {
447+
// Grow the layout bounds (same object attachPanZoom fits) so Fit/double-click
448+
// can still frame a node dragged past dagre's original extent.
449+
if (x + node.w > built.width) built.width = x + node.w;
450+
if (y + node.h > built.height) built.height = y + node.h;
451+
for (const i of incidentById.get(id)) { // every node id is mapped above
415452
const ed = edges[i];
416453
const pts = straightEdgePoints(byId.get(ed.from), byId.get(ed.to));
417454
pathByIdx.get(i).setAttribute('d', 'M' + pts.map((p) => p.x + ' ' + p.y).join(' L'));
455+
// Keep the relationship label on the re-routed edge's midpoint, not stranded.
456+
const lbl = labelByIdx.get(i);
457+
if (lbl) { lbl.setAttribute('x', (pts[0].x + pts[1].x) / 2); lbl.setAttribute('y', (pts[0].y + pts[1].y) / 2 - 3); }
418458
}
419459
if (positions) recordPosition(positions, id, x, y);
420460
};
@@ -431,22 +471,31 @@ function attachSchemaInteractions(canvas, svg, built, targetDoc, positions, onCh
431471
else if (k === 'y') { e.preventDefault(); doRedo(); } // ⌘Y redo (Windows-style)
432472
};
433473
const onKeyUp = (e) => { if (!(e.metaKey || e.ctrlKey)) canvas.classList.remove('modkey'); };
474+
// If the window loses focus mid-press the modifier keyup may never arrive, which
475+
// would leave the grab/move cursor (.modkey) latched on — clear it on blur.
476+
const onBlur = () => canvas.classList.remove('modkey');
477+
const win = targetDoc.defaultView;
434478
const onDown = (e) => {
435-
if (!(e.metaKey || e.ctrlKey)) return; // plain drag → let the pan handler have it
436479
const g = e.target.closest('[data-node-id]');
437-
if (!g) return;
480+
if (!(e.metaKey || e.ctrlKey)) {
481+
// Plain press on a card: swallow it so the canvas doesn't pan (a clean click
482+
// still opens the detail pane). Plain press on empty canvas falls through to pan.
483+
if (g) e.stopPropagation();
484+
return;
485+
}
486+
if (!g) return; // ⌘/Ctrl on empty canvas → let the pan handler grab it
438487
const node = byId.get(g.getAttribute('data-node-id'));
439488
if (!node) return;
440489
e.preventDefault(); e.stopPropagation();
441490
canvas.classList.add('grabbing');
442491
const start = { x: node.x, y: node.y }; // for the undo record
443-
// The viewBox and the container box are fixed for the duration of a node drag,
444-
// so read them once here instead of reflowing/parsing on every mousemove.
445-
const vb = getVb();
492+
// The container box is stable for the drag, so read it once; the viewBox is
493+
// re-read each move (a ⌘/wheel zoom mid-drag changes it) so deltas stay scaled.
446494
const rect = canvas.getBoundingClientRect();
447495
let last = { x: e.clientX, y: e.clientY };
448496
const onMove = (ev) => {
449-
const { dx, dy } = dragDeltaToSvg(ev.clientX - last.x, ev.clientY - last.y, vb, rect);
497+
if (ev.buttons === 0) return onUp(); // button released off-window → end the drag
498+
const { dx, dy } = dragDeltaToSvg(ev.clientX - last.x, ev.clientY - last.y, getVb(), rect);
450499
last = { x: ev.clientX, y: ev.clientY };
451500
placeAt(node.id, node.x + dx, node.y + dy);
452501
};
@@ -463,17 +512,19 @@ function attachSchemaInteractions(canvas, svg, built, targetDoc, positions, onCh
463512
targetDoc.addEventListener('keydown', onKeyDown);
464513
targetDoc.addEventListener('keyup', onKeyUp);
465514
canvas.addEventListener('mousedown', onDown, true);
515+
if (win) win.addEventListener('blur', onBlur);
466516
return {
467517
undo: doUndo,
468518
redo: doRedo,
469519
canUndo: () => history.canUndo(),
470520
canRedo: () => history.canRedo(),
471-
// Teardown: the overlay path attaches keydown/keyup to the persistent main
472-
// document, so closing must remove them (the tab path drops them with its doc).
521+
// Teardown: the overlay path attaches keydown/keyup/blur to the persistent main
522+
// document/window, so closing must remove them (the tab path drops them with its doc).
473523
teardown: () => {
474524
targetDoc.removeEventListener('keydown', onKeyDown);
475525
targetDoc.removeEventListener('keyup', onKeyUp);
476526
canvas.removeEventListener('mousedown', onDown, true);
527+
if (win) win.removeEventListener('blur', onBlur);
477528
},
478529
};
479530
}
@@ -484,8 +535,10 @@ function attachSchemaInteractions(canvas, svg, built, targetDoc, positions, onCh
484535
// error in the canvas and toasts the main window.
485536
function makeController(app, targetDoc, mainDoc, canvas, bar, closeBtn) {
486537
let teardown = null;
538+
let destroyed = false;
487539
return {
488540
render(graph) {
541+
if (destroyed) return; // the view was closed before the lineage finished loading
489542
withDocument(targetDoc, () => {
490543
canvas.textContent = '';
491544
bar.querySelector('.graph-overlay-title').textContent = 'Schema: ' + focusLabel(graph.focus);
@@ -494,13 +547,16 @@ function makeController(app, targetDoc, mainDoc, canvas, bar, closeBtn) {
494547
if (targetDoc !== mainDoc) targetDoc.title = 'Schema:' + focusLabel(graph.focus);
495548
const built = buildRichSchemaSvg(graph, app.Dagre, schemaDetailClick(app, targetDoc));
496549
// Right-aligned action cluster: theme switcher + (zoom controls) + (close).
497-
const actions = h('div', { class: 'graph-overlay-actions' }, themeToggle(targetDoc));
550+
// In the overlay (targetDoc === mainDoc) the toggle routes through app's own
551+
// toggleTheme so state/pref/header stay in sync; a real tab flips locally.
552+
const actions = h('div', { class: 'graph-overlay-actions' },
553+
themeToggle(targetDoc, targetDoc === mainDoc ? app.toggleTheme : null));
498554
if (!built.nodeCount) {
499555
canvas.appendChild(placeholder(schemaEmptyMessage(graph)));
500556
} else {
501557
canvas.classList.add('schema-canvas');
502558
canvas.appendChild(built.svg);
503-
const pz = attachPanZoom(canvas, built.svg, built, { fitWidth: true });
559+
const pz = attachPanZoom(canvas, built.svg, built, { fitWidth: true, refitOnResize: true });
504560
let undoBtn, redoBtn;
505561
const refresh = () => { undoBtn.disabled = !controls.canUndo(); redoBtn.disabled = !controls.canRedo(); };
506562
const controls = attachSchemaInteractions(canvas, built.svg, built, targetDoc, graph.savedPositions, refresh);
@@ -520,10 +576,11 @@ function makeController(app, targetDoc, mainDoc, canvas, bar, closeBtn) {
520576
});
521577
},
522578
fail(msg) {
579+
if (destroyed) return;
523580
withDocument(targetDoc, () => { canvas.textContent = ''; canvas.appendChild(placeholder(msg)); });
524581
flashToast(msg, { document: mainDoc });
525582
},
526-
destroy() { if (teardown) teardown(); },
583+
destroy() { destroyed = true; if (teardown) teardown(); },
527584
};
528585
}
529586

tests/unit/app.test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,10 @@ describe('renderApp shell', () => {
124124
});
125125
it('toggles theme via the header button', () => {
126126
const { app } = rendered();
127-
app.dom.themeBtn.dispatchEvent(new Event('click'));
128-
expect(app.state.theme).toBe('light');
127+
app.dom.themeBtn.dispatchEvent(new Event('click')); // default light → dark
128+
expect(app.state.theme).toBe('dark');
129129
expect(app.savePref).toBeUndefined; // savePref is internal; theme attr set
130-
expect(document.documentElement.getAttribute('data-theme')).toBe('light');
130+
expect(document.documentElement.getAttribute('data-theme')).toBe('dark');
131131
});
132132
it('user menu: open → Log out clears tokens and shows login', () => {
133133
const { app, e } = rendered();

0 commit comments

Comments
 (0)