fix(schema): clear the selection ring on every pane-close path (review #65)

Address the medium /code-review findings on this PR:

- Esc-closing the detail pane (the schema tab + the in-app overlay handlers in
  explain-graph.js) did `pane.remove()` directly, bypassing the ✕ button's
  cleanup — leaving the accent ring + .eg-card--selected class orphaned on the
  card with no pane open. Export `clearSchemaSelection(doc)` and call it from both
  Esc paths so every close route clears the selection. (the real bug)
- Simplify clearSchemaSelection to a single scan: the ring is always a child of
  the selected card, so strip the class and remove its ring together.
- Fix two now-stale comments (schemaDetailClick + openDetailPane) that still
  referenced the removed "Insert SHOW CREATE" button / old close behavior.
- Drop the dead `insertCreate: vi.fn()` stubs from two tests (and the now-unused
  `vi` import) — the detail pane no longer calls insertCreate.

Extends the overlay Esc test to assert the ring is cleared. 1023 tests pass;
schema-detail.js stays at 100% coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019kE9qbgBNBrfNgwg9fRsMJ

Author: BorisTyshkevich

src/ui/explain-graph.js

@@ -14,6 +14,7 @@ import { qualifyIdent } from '../core/format.js';
 import { fitBox, fitWidthBox, zoomBox, panBox, viewBoxStr } from '../core/panzoom.js';
 import { straightEdgePoints, incidentEdges, dragDeltaToSvg, applyPositions, recordPosition, createMoveHistory } from '../core/graph-layout.js';
 import { flashToast } from './toast.js';
+import { clearSchemaSelection } from './schema-detail.js';
 
 const ZOOM_STEP = 1.2; // per zoom-button press
 const WHEEL_ZOOM_STEP = 1.04; // per ⌘/Ctrl+wheel notch — gentle, so trackpad/wheel zoom isn't jumpy
@@ -344,9 +345,9 @@ const schemaClick = (app) => (n) => {
 };
 
 // In the full schema view, clicking an object opens the detail pane (full columns
-// / keys / partitions / DDL) instead of inserting SHOW CREATE — the pane carries
-// its own "Insert SHOW CREATE" button. External (ext:) leaves have no detail; a
-// ⌘/Ctrl-click is reserved for dragging the node, so it doesn't open the pane.
+// / keys / partitions / DDL) instead of inserting SHOW CREATE, and rings the
+// clicked card. External (ext:) leaves have no detail; a ⌘/Ctrl-click is reserved
+// for dragging the node, so it doesn't open the pane.
 // `targetDoc` is this view's own document (the tab or the overlay's host), threaded
 // so a node click always opens the pane in the view it came from — even when
 // several full views are open at once (no shared single-slot document).
@@ -601,7 +602,7 @@ function openInTab(app, win, childDoc, mainDoc) {
     childDoc.addEventListener('keydown', (e) => {
       if (e.key !== 'Escape') return;
       const pane = childDoc.querySelector('.schema-detail');
-      if (pane) { e.stopPropagation(); pane.remove(); }
+      if (pane) { e.stopPropagation(); pane.remove(); clearSchemaSelection(childDoc); }
     }, true);
     return makeController(app, childDoc, mainDoc, canvas, bar, null);
   });
@@ -617,7 +618,7 @@ function openInOverlay(app, mainDoc) {
       if (e.key !== 'Escape') return;
       e.stopPropagation();
       const pane = mainDoc.querySelector('.schema-detail');
-      if (pane) pane.remove(); else close();
+      if (pane) { pane.remove(); clearSchemaSelection(mainDoc); } else close();
     };
     let backdrop;
     // close() also tears down the interaction listeners attached to the main

src/ui/schema-detail.js

@@ -16,8 +16,9 @@ const TOP_MARGIN = 100;
 /**
  * Mount (or replace) the detail pane for `node` inside the live fullscreen overlay,
  * populated from `detail` ({ columns, partitions, ddl }). Returns the pane element,
- * or null when no overlay is open. The ✕ button closes just the pane; Esc closes
- * the whole overlay (which removes the pane with it).
+ * or null when no overlay is open. The ✕ button and Esc both close just the pane
+ * and clear the card's selection ring (Esc is wired in explain-graph.js via the
+ * exported clearSchemaSelection); a further Esc / backdrop click closes the view.
  */
 export function openDetailPane(app, node, detail, targetDoc) {
   // `targetDoc` is the view's own document (a schema tab, or the overlay's host);
@@ -41,18 +42,24 @@ function findCard(doc, nodeId) {
   return [...doc.querySelectorAll('.eg-card[data-node-id]')].find((g) => g.getAttribute('data-node-id') === nodeId) || null;
 }
 
-// Clear any prior selection: drop the marker class and remove the ring rects.
-function clearSelection(doc) {
-  doc.querySelectorAll('.eg-card--selected').forEach((g) => g.classList.remove('eg-card--selected'));
-  doc.querySelectorAll('.eg-card-ring').forEach((ring) => ring.remove());
+// Clear the selection highlight in `doc`: drop the marker class and its ring rect
+// from the selected card (the ring is always a child of that card). Exported so the
+// graph's other pane-close paths — Esc in the schema tab / in-app overlay, in
+// explain-graph.js — clear it too, not only the pane's own ✕ button.
+export function clearSchemaSelection(doc) {
+  doc.querySelectorAll('.eg-card--selected').forEach((g) => {
+    g.classList.remove('eg-card--selected');
+    const ring = g.querySelector('.eg-card-ring');
+    if (ring) ring.remove();
+  });
 }
 
 // Mark `nodeId`'s card as selected: an accent ring drawn just outside its box (a
 // "double border" alongside the card's own kind-coloured stroke) plus a class the
 // CSS keys off. Replaces any prior selection. No-op when the card isn't drawn
 // (e.g. the pane opened over a view without that card, or in a test harness).
 function markSelected(doc, nodeId) {
-  clearSelection(doc);
+  clearSchemaSelection(doc);
   const card = findCard(doc, nodeId);
   if (!card) return;
   card.classList.add('eg-card--selected');
@@ -98,7 +105,7 @@ function buildDetailPane(node, detail, panel) {
   const handle = h('div', { class: 'schema-detail-handle', title: 'Drag to resize' });
   const pane = h('div', { class: 'schema-detail' },
     handle,
-    h('button', { class: 'schema-detail-close', title: 'Close', onclick: () => { pane.remove(); clearSelection(doc); } }, Icon.close()),
+    h('button', { class: 'schema-detail-close', title: 'Close', onclick: () => { pane.remove(); clearSchemaSelection(doc); } }, Icon.close()),
     h('div', { class: 'schema-detail-body' },
       h('div', { class: 'schema-detail-head' },
         h('b', null, ident), h('span', { class: 'schema-detail-kind' }, node.kind || 'table')),

tests/unit/explain-graph.test.js

@@ -287,14 +287,23 @@ describe('schema lineage graph', () => {
     expect(document.body.contains(overlay)).toBe(false);
   });
 
-  it('Esc closes the open detail pane first, then the overlay', () => {
+  it('Esc closes the open detail pane first (clearing its card ring), then the overlay', () => {
     openSchemaView(overlayApp({ openNodeDetail: vi.fn() })).render(GRAPH);
     const overlay = overlayOf();
+    const panel = overlay.querySelector('.graph-overlay-panel');
     const pane = document.createElement('div'); pane.className = 'schema-detail';
-    overlay.querySelector('.graph-overlay-panel').appendChild(pane);
+    panel.appendChild(pane);
+    // a selected card with a ring, as markSelected would have left it on the canvas
+    const card = document.createElementNS('http://www.w3.org/2000/svg', 'g');
+    card.setAttribute('class', 'eg-card eg-card--selected');
+    card.setAttribute('data-node-id', 'x.y');
+    card.appendChild(document.createElementNS('http://www.w3.org/2000/svg', 'rect')).setAttribute('class', 'eg-card-ring');
+    panel.appendChild(card);
     document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }));
-    expect(overlay.querySelector('.schema-detail')).toBeNull(); // pane closed
-    expect(document.body.contains(overlay)).toBe(true); // overlay stays
+    expect(overlay.querySelector('.schema-detail')).toBeNull();      // pane closed
+    expect(overlay.querySelector('.eg-card--selected')).toBeNull();  // selection class cleared
+    expect(overlay.querySelector('.eg-card-ring')).toBeNull();       // ring removed
+    expect(document.body.contains(overlay)).toBe(true);              // overlay stays
     document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }));
     expect(document.body.contains(overlay)).toBe(false); // second Esc closes overlay
   });

tests/unit/schema-detail.test.js

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, afterEach } from 'vitest';
+import { describe, it, expect, afterEach } from 'vitest';
 import { openDetailPane } from '../../src/ui/schema-detail.js';
 
 afterEach(() => { document.body.innerHTML = ''; });
@@ -157,15 +157,15 @@ describe('openDetailPane', () => {
     const panel = childDoc.createElement('div');
     panel.className = 'graph-overlay-panel';
     childDoc.body.appendChild(panel);
-    const pane = openDetailPane({ document, actions: { insertCreate: vi.fn() } }, NODE, DETAIL, childDoc);
+    const pane = openDetailPane({ document, actions: {} }, NODE, DETAIL, childDoc);
     expect(pane.ownerDocument).toBe(childDoc); // built in the child tab's document
     expect(childDoc.querySelector('.schema-detail')).not.toBeNull();
     expect(document.querySelector('.schema-detail')).toBeNull(); // not in the main document
   });
 
   it('falls back to the global document and tolerates missing columns/partitions', () => {
     mountPanel();
-    const pane = openDetailPane({ actions: { insertCreate: vi.fn() } }, NODE, { ddl: '' }); // no document/detailDocument
+    const pane = openDetailPane({ actions: {} }, NODE, { ddl: '' }); // no document/detailDocument
     expect(pane).not.toBeNull();
     expect([...pane.querySelectorAll('h4')].map((e) => e.textContent)).toEqual(['Columns (0)']);
   });