fix(#568): Dashboard tree — single revealed action cluster, inline count

CHANGELOG.md

@@ -25,6 +25,26 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
   adapter refactor — no user-visible behavior changes.
 
 ### Fixed
+- **The Dashboard tree no longer reveals two rows' pencil/trash actions at
+  once, and its `· N` count now sits inline after the label** (#568). The
+  hover/focus reveal rule (`.dash-tree-row:focus-within .dash-tree-act`)
+  matched on any focus, so a row a user had merely clicked to select kept its
+  actions visible indefinitely, alongside whichever other row the pointer was
+  hovering — swapped to `:focus-visible`/`:has(:focus-visible)` (the row's own
+  composite tab stop and its controls, respectively — `:has()`'s implicit
+  descendant selector alone misses the row itself), which only a real
+  keyboard Tab/Arrow grants, preserving reveal-on-keyboard-focus without
+  pinning a clicked row open. A directly-focused action button (`.dash-tree-
+  act:focus`, no `:focus-visible`) reveals only itself regardless of
+  modality, so a programmatic focus-return or a test's direct `.focus()` —
+  the pre-existing contract `tests/e2e/tile-open-workbench.spec.js`'s
+  "Panels-row plus" test already relied on — still works. Separately,
+  `Sales revenue · 2` read as a
+  right-aligned
+  column rather than immediately after the name (`Library · 66`'s placement,
+  as `dashboard-tree-model.ts` already documented as the intent): the label
+  and count are now wrapped in one flex group so only that group grows,
+  matching the narrow-sidebar test's own claim.
 - **A detached Data pane's variable dropdown no longer shows a frozen
   snapshot of recent values** (#478 follow-up). #555's caller-neutral
   `VariableBarApp` adapter copied `varRecent` into `state` as a plain data

src/styles.css

@@ -1720,7 +1720,13 @@ body.detached-tab .graph-overlay-panel {
 .dash-tree-row { position: relative; }
 /* The group rows (Variables / Panels) are structure, not content. */
 .dash-tree-group { color: var(--fg-faint); font-weight: var(--fw-medium); }
-.dash-tree-count { margin-left: 2px; font-size: var(--text-label); }
+/* `flex-shrink: 0` + `white-space: nowrap`: inside `.dash-tree-label-group`
+   below, an unbounded `.label` (its flex-basis is its full, untruncated text
+   width) would otherwise absorb almost none of the shrink budget itself,
+   leaving this tiny "· N" to take the rest — squeezing it down to a couple of
+   pixels and wrapping "· 2" onto two lines. The count stays fixed-width; the
+   label does all the shrinking, same as it always ellipsizes. */
+.dash-tree-count { margin-left: 2px; font-size: var(--text-label); flex-shrink: 0; white-space: nowrap; }
 /* CURRENT RESOURCE vs KEYBOARD FOCUS must read differently (#426): the current
    Dashboard/member is a persistent tonal surface with an accent edge, while
    keyboard focus is the transient ring. A row can legitimately be both. */
@@ -1810,15 +1816,43 @@ body.detached-tab .graph-overlay-panel {
   opacity: 0; pointer-events: none;
 }
 /* Revealed on hover like the Library row actions, but ALWAYS in the keyboard
-   focus order: `:focus-within` makes the visually concealed cluster visible
-   while the row or one of its controls owns focus, so no operation is
-   keyboard-unreachable.
+   focus order: `:focus-visible`/`:has(:focus-visible)` make the visually
+   concealed cluster visible while the row itself OR one of its controls holds
+   KEYBOARD focus, so no operation is keyboard-unreachable. Both are needed —
+   `:has()`'s argument is an implicit DESCENDANT selector, so `:has(:focus-
+   visible)` alone matches only when a control inside the row (the chevron or
+   an action button) is focused, not when the row's own composite tab stop
+   is (the roving-tabindex owner a real ArrowDown/ArrowUp or the first Tab
+   into the tree lands on) — that gap left the cluster invisible at exactly
+   the moment #472 wants a sighted keyboard user to see it waiting.
+   Deliberately not `:focus-within`, which matches on any focus regardless of
+   modality — a row a user merely CLICKED to select keeps DOM focus, so
+   `:focus-within` kept its actions revealed forever, alongside whichever row
+   the pointer was hovering, showing two pencil/trash pairs at once. Ordinary
+   pointer focus on the ROW no longer pins it open this way — though a
+   different row's genuine hover and this row's genuine keyboard focus can
+   still be visible at once, which is correct: those are two independent,
+   live states, not the stale-selection bug. Chromium (and other engines)
+   withhold `:focus-visible` after a pointer interaction but grant it for real
+   keyboard navigation — Tab/Arrow keys — even when the resulting focus is set
+   programmatically from a keydown handler (see the Tab-walk and
+   keyboard-delete tests in `tests/e2e/dashboard-tree.spec.js`).
+   `.dash-tree-act:focus` (plain, not `:focus-visible`) is separate and
+   narrower: the button ITSELF holding any focus — Tab, a programmatic
+   focus-return once its own dialog closes, or a test's direct `.focus()` —
+   reveals only that one control, never a sibling or the whole row, so it
+   cannot resurrect the row-level bug above. This preserves the pre-existing
+   contract `tests/e2e/tile-open-workbench.spec.js`'s "Panels-row plus" test
+   and its sibling `.dash-tile-actions` ("Focus alone reveals it — a keyboard
+   user never hovers") already rely on.
    `[aria-expanded="true"]` holds it visible for its own dialog/confirmation's
    whole lifetime — both are body-mounted, so by the time one closes the
    pointer and focus have left the row, and an invisible trigger must not
    become unreachable before focus can return to it. */
 .dash-tree-row:hover .dash-tree-act,
-.dash-tree-row:focus-within .dash-tree-act,
+.dash-tree-row:focus-visible .dash-tree-act,
+.dash-tree-row:has(:focus-visible) .dash-tree-act,
+.dash-tree-act:focus,
 .dash-tree-act[aria-expanded="true"] { opacity: 1; pointer-events: auto; }
 /* #447's orphan-variable trash stays ALWAYS visible: it is the only way to
    remove stored SQL nothing references any more, and a hover-only affordance
@@ -1914,6 +1948,16 @@ body.detached-tab .graph-overlay-panel {
 }
 .tree-row .meta.loading { font-style: italic; opacity: .6; }
 
+/* Dashboard tree count reads immediately after the label — matching Library's
+   `Library · 66` placement — instead of inheriting the schema tree's
+   flex:1 label above, which pushes trailing content to the row's far right
+   edge. Only the GROUP grows; the label inside it shrinks/ellipsizes and the
+   count stays a fixed-width neighbor. Placed after `.tree-row .label` above
+   so this wins the `flex` override at equal selector specificity, without
+   touching the schema (Databases) tree's own `.tree-row .label` behavior. */
+.dash-tree-label-group { display: flex; align-items: center; min-width: 0; flex: 1; }
+.dash-tree-label-group .label { flex: 0 1 auto; min-width: 0; }
+
 /* Column name/type independent drag targets (#186). Only these two child
    spans are drag sources on a column row — the row-level `:active grabbing`
    above no longer applies to columns (overridden just below, scoped to `.mono`

src/ui/dashboard-tree.ts

@@ -576,7 +576,7 @@ function dropProps(app: DashboardTreeApp, row: DashboardTreeRow): Record<string,
     ondragenter: over,
     ondragover: over,
     ondragleave: (event: DragEvent) => {
-      // The row has eight child spans and drag events bubble, so a bare
+      // The row has several child spans and drag events bubble, so a bare
       // `dragleave` fires every time the pointer crosses onto one of them.
       const to = event.relatedTarget;
       const rowEl = event.currentTarget as HTMLElement;
@@ -674,6 +674,11 @@ function buildRow(
   const count = row.count === null
     ? null
     : h('span', { class: 'side-count dash-tree-count' }, '· ' + row.count);
+  // The label and its count grow as ONE group, so the count reads right after
+  // the name (`Sales revenue · 2`) like the Library tab's `Library · 66` —
+  // rather than as a sibling of a flex:1 label, which pushes it into the
+  // right-aligned cluster with `meta`/actions.
+  const labelGroup = h('span', { class: 'dash-tree-label-group' }, label, count);
   // #447: the WORD, not only a colour — an unused (orphaned) variable says so in
   // text, right after its name.
   const status = row.invalid === 'variable-unused'
@@ -737,7 +742,7 @@ function buildRow(
       syncRovingTabindex(rowEl.parentElement, row.key);
       pressRow(app, row, event.shiftKey);
     },
-  }, chevron, h('span', { class: 'icon' }, rowIcon(row)), label, count, status,
+  }, chevron, h('span', { class: 'icon' }, rowIcon(row)), labelGroup, status,
     h('span', { class: 'meta' }, row.meta), marker,
     // #494: the trailing DIRECT controls, in the model's own order — edit
     // before delete, destructive rightmost. No tree ROW carries a `⋯` any more.

tests/e2e/dashboard-tree.spec.js

@@ -921,6 +921,64 @@ test.describe('direct row actions (#494)', () => {
     await expect(row.getByRole('button', { name: 'Delete dashboard Ops latency' })).toBeFocused();
   });
 
+  // The bug this guards: `:focus-within` reveals a row's actions on ANY
+  // focus, including the focus a plain CLICK leaves behind on the row it
+  // selected — so that row's pencil/trash stayed revealed indefinitely,
+  // alongside whichever OTHER row the pointer moved on to hover next. Only
+  // real keyboard focus (`:focus-visible`) should pin a row's actions open
+  // once the pointer has left it.
+  test('clicking a row to select it does not leave its actions revealed once the pointer hovers another row', async ({ page }) => {
+    await open(page);
+    await roleTab(page, 'Dashboards').click();
+    const clicked = treeRow(page, 'workspace:ops');
+    const hovered = treeRow(page, 'workspace:long');
+    await clicked.click();
+    await hovered.hover();
+    await expect(clicked.locator('.dash-tree-act').first()).toHaveCSS('opacity', '0');
+    await expect(hovered.locator('.dash-tree-act').first()).toHaveCSS('opacity', '1');
+  });
+
+  // `:has()`'s argument is an implicit DESCENDANT selector, so a reveal rule
+  // written as only `.dash-tree-row:has(:focus-visible)` would miss the row's
+  // OWN composite tab stop — the roving-tabindex owner a real ArrowDown/
+  // ArrowUp or the first Tab into the tree lands ON, before a chevron or
+  // action button has been reached. That left the cluster invisible at
+  // exactly the moment a sighted keyboard user first arrives at a row.
+  test('arrowing onto a row reveals its own actions, not just a descendant control\'s', async ({ page }) => {
+    await open(page);
+    await roleTab(page, 'Dashboards').click();
+    const clicked = treeRow(page, 'workspace:sales');
+    const next = treeRow(page, 'workspace:ops');
+    // Establish POINTER modality first — a real click, the way a user actually
+    // lands keyboard focus on a row in practice — so the ArrowDown below is
+    // the thing that flips modality to keyboard, not a leftover from `open()`'s
+    // own setup. This isolates the historical cross-engine failure mode: a
+    // pointer→keyboard transition that lands on a DIFFERENT row entirely.
+    await clicked.click();
+    expect(await clicked.evaluate((el) => el.matches(':focus-visible'))).toBe(false);
+    await page.keyboard.press('ArrowDown');
+    await expect(next).toBeFocused();
+    expect(await next.evaluate((el) => el.matches(':focus-visible'))).toBe(true);
+    await expect(next.locator('.dash-tree-act').first()).toHaveCSS('opacity', '1');
+  });
+
+  // The DESCENDANT half of the reveal rule — `.dash-tree-row:has(:focus-
+  // visible)` — is exercised separately from the row's own composite tab stop
+  // above: Tab onto the chevron (a control, not the row, and not an action
+  // button, so `.dash-tree-act:focus` cannot be why this passes) and confirm
+  // the cluster stays visible with the pointer elsewhere.
+  test('tabbing onto the chevron reveals the row\'s actions via the descendant :has() branch', async ({ page }) => {
+    await open(page);
+    await roleTab(page, 'Dashboards').click();
+    const row = treeRow(page, 'workspace:sales');
+    await row.focus();
+    await page.keyboard.press('Tab');
+    const chevron = row.locator('.dash-tree-chev');
+    await expect(chevron).toBeFocused();
+    expect(await chevron.evaluate((el) => el.matches(':focus-visible'))).toBe(true);
+    await expect(row.locator('.dash-tree-act').first()).toHaveCSS('opacity', '1');
+  });
+
   test('the dialog announces itself as a modal named by its heading', async ({ page }) => {
     await open(page);
     await roleTab(page, 'Dashboards').click();
@@ -1125,6 +1183,23 @@ test.describe('Dashboard tree counts at the narrow sidebar (#553)', () => {
     for (const key of ['workspace:sales', 'workspace:sales:group:variables', 'workspace:sales:group:panels']) {
       await expect(treeRow(page, key).locator('.dash-tree-count')).toBeVisible();
     }
+    // "Inline after the label" is a geometric claim, not just a text/visibility
+    // one: the count must sit right next to the label's own box, not pushed
+    // into the row's right-aligned trailing cluster (meta/marker/actions). The
+    // intended gap is the count's own 2px margin; the OLD (buggy) layout's gap
+    // was exactly `.tree-row`'s 6px flex `gap` PLUS that 2px margin = 8px, so
+    // the upper bound sits well clear of 8 rather than grazing it (a boundary
+    // right on the old value risks a sub-pixel rounding false pass), and a
+    // lower bound catches the count overlapping the label instead of merely
+    // failing to be far away from it.
+    for (const key of ['workspace:sales', 'workspace:sales:group:variables', 'workspace:sales:group:panels']) {
+      const row = treeRow(page, key);
+      const labelBox = await row.locator('.label').boundingBox();
+      const countBox = await row.locator('.dash-tree-count').boundingBox();
+      const gap = countBox.x - (labelBox.x + labelBox.width);
+      expect(gap).toBeGreaterThanOrEqual(-0.5);
+      expect(gap).toBeLessThan(5);
+    }
   });
 
   test('dragging to <=220px hides every dot/count, but the count stays in the accessible name and actions stay reachable', async ({ page }) => {