Skip to content

feat(#429): split the Dashboard tree row into chevron, name and trailing targets (phase 2) - #492

Merged
BorisTyshkevich merged 3 commits into
mainfrom
feat/dashboard-row-gestures-429p2
Jul 27, 2026
Merged

feat(#429): split the Dashboard tree row into chevron, name and trailing targets (phase 2)#492
BorisTyshkevich merged 3 commits into
mainfrom
feat/dashboard-row-gestures-429p2

Conversation

@BorisTyshkevich

@BorisTyshkevich BorisTyshkevich commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

What & why

Part of #429phase 2, which absorbs #472: the Dashboard-tree row becomes three
independent targets
.

Target Action
Disclosure chevron (now a real <button>) expand / collapse this Dashboard's hierarchy, and nothing else
Dashboard name / primary row content (icon included) open in View, immediately
Shift-click on the name open Edit
Trailing acts without navigating or expanding

Before this, a Dashboard row's single click only expanded it — deferred ~300ms through
the click arbiter — and opening needed a double-click. Expansion and navigation competed
for one gesture; now they don't, so the arbitration is gone from Dashboard rows entirely.
A panel row is untouched: click opens its query, double-click and Shift-click focus
the tile, and it still arbitrates.

The decision this phase owed the issue

"Once the name target opens View and Shift-click opens Edit, the Dashboard row's
Open in View / Open in Edit menu items are redundant. Either drop them or keep them
deliberately … and say which."

Split, not all-or-nothing:

  • Dropped Open in View — it is the row's primary press now (pointer and Enter),
    so nothing about it is hidden any more, which was the menu item's only justification.
  • Kept Open in Edit — its only forms are Shift-click and Shift+Enter, both hidden
    modifier gestures. The menu is where a keyboard or first-time user finds Edit mode at
    all; dropping it would trade a fixed redundancy for a real discoverability regression.

That also leaves the Dashboard row with one trailing control going into phase 3, which
is what the issue's "a third icon on a narrow sidebar row is the reason to think twice"
warns about — phase 3 can still choose a pencil icon or a second menu row.

Implementation notes

  • dashboard-tree-model.ts: the Dashboard row emits single: open-dashboard(view),
    double: null, and keeps shift. double === null is the view's new arbitration
    switch
    — no new field. single is now unconditional, so a Dashboard the search is
    holding open can be opened (it still cannot be collapsed — that is toggleable),
    where before that click did nothing at all.
  • The chevron carries aria-expanded, an Expand <name> / Collapse <name> accessible
    name, native Enter/Space that is stopped from reaching the tree's own Enter (which now
    navigates), a focus ring in a different channel from the row's, and a roving
    tabindex so the tree stays one composite tab stop instead of one stop per chevron.
    The treeitem keeps its own aria-expanded; both are painted from row.expanded.
  • Keyboard activation returns focus to the chevron across the repaint its own toggle
    causes — otherwise one Space moved focus to the row and the next Enter navigated.
  • Three guards the split made unreachable were removed, not left as dead code: the
    chevron's cancelFor, the Add Library drag assignment to Dashboard panels and variables #428 drop handler's cancelFor (only a row with a double
    is ever arbitrated → a panel row → which accepts no drop and has no chevron), and
    toggleRow's Dashboard arm (the model now emits {kind:'toggle'} for group rows only).
  • No duplicate history entry on a double-click needed no new tree code: openDashboard
    already collapses a repeated same-id/same-mode open. That guarantee is now pinned by a
    test in tests/unit/app.test.ts rather than assumed — the tree deliberately does not
    duplicate the guard, since it cannot see the route.

The bug only a real browser could show (second commit)

Hit-testing and getBoundingClientRect use the transformed box. The new button is
10×24 (stretched to the row height so a 10px glyph is worth aiming at), so a collapsed
row's rotate(-90deg) turned its clickable band into 24 wide × 10 tall — spilling 7px
each side and swallowing clicks meant for the row icon, which then expanded instead of
opening. The schema tree's chevron survives rotating its own box only because it is a
10×10 square. The rotation now lives on the glyph inside the control, and an e2e case
asserts the control's box stays in its own slot and never reaches the icon (verified to
fail with the rotation put back on the button). happy-dom has no layout, so nothing below
e2e could hold this.

The accessibility regression the review pass caught (third commit)

A treeitem names itself from its contents, and those contents now include a
labelled button. Measured with Chromium's own accname engine
(Accessibility.getPartialAXTree), not assumed:

collapsed   "Expand Sales revenue Sales revenue 2"
expanded    "Collapse Sales revenue Sales revenue 2 Actions for Sales revenue"

The chevron's name was being folded into the row's — the opposite of the criterion this
PR claims. The leak was pre-existing and only while hovered/focused; the permanent
Expand … prefix on every Dashboard and group row was this phase's regression.

The row now states its own name, composed from the model's strings in the order it paints
them (name, count, status word, meta, marker label), so it cannot drift from the screen
and nothing previously announced is lost. Held by exact-name unit assertions and by
getByRole in e2e — which resolves real accessible names in every engine, where
happy-dom cannot compute one at all. Sabotage-checked: dropping the attribute fails both.

Two smaller review items came with it: the fourth corner of #472's matrix (expanded
and current) is now covered, a real dblclick is asserted to dispatch one repeated
command, and a CSS comment that called a three-class selector "equal-specificity" is
corrected.

Acceptance criteria claimed

Deferred by design: the Dashboard pencil and the one create path (phase 3), panel
metadata edit (4), atomic panel+query delete and ownership (5), the repair planner (6).
The ## Regressions bullet "row click/double-click/Shift-click contract remains
unchanged"
is scoped to the rows this issue does not revise — #472's own section
overrides it for Dashboard rows, and panel/variable/group rows are genuinely untouched.
The non-goal "Moving or redesigning the trailing Dashboard action menu as a control" is
respected: the control is unchanged, only one item of its contents went.

#438 / layout engines: phase 2 adds no tile-focus gesture — a Dashboard row navigates
to a Dashboard, never to a member — so there was nothing to verify on the flow engine. The
obligation stands for phases 4/5.

Verification

  • npm test5827 passed / 177 files, per-file gate held; tsc --noEmit clean;
    npm run build clean.
  • npm run test:e2e442 passed, 0 failed on chromium + webkit + firefox.
    (Firefox launches here again after npx playwright install firefox; the dependabot
    Playwright bump had left all three browser binaries stale, which reads as ~435 failures
    at 1ms each.)
  • An independent read-only audit against the issue's phase-2 contract came back clean on
    under-delivery, scope creep, non-goal breaches and rule-softening; its one substantive
    finding is the accessible-name regression fixed above.
  • Sabotage-checked, six ways: letting Enter/Space bubble past the chevron, dropping the
    chevron focus restore, not moving the keyboard owner, making every chevron tabbable,
    reverting the model split, and restoring Open in View — each is caught by 1–6 tests and
    nothing else. The repeated-open history guard and the chevron box were sabotage-checked
    too, as was the accessible name.
  • Driven in the real served app (not the e2e fixture) against a live ClickHouse: the
    chevron is a BUTTON labelled Expand Sales revenue with aria-expanded=false and
    tabindex=0; Tab walks row → chevron → ; the three focus treatments are a box-shadow
    ring, a 2px accent outline and the trailing button's own; a chevron click expands 2 → 4
    rows without touching the URL; a name click routes to
    ?surface=dashboard&mode=view. No page errors.

Checklist

  • npm test passes (the per-file coverage gate is non-negotiable)
  • Tests added/updated in the same change as the code
  • npm run build succeeds (single-file dist/sql.html)
  • Layers kept honest: pure logic in src/core/, network in src/net/ (injected fetch), DOM in src/ui/
  • No new runtime dependency (or it's a deliberate, justified addition — see CONTRIBUTING)
  • README / CHANGELOG.md ([Unreleased]) updated if behavior or the deployed surface changed
  • Reconciled affected tracked work (roadmap Roadmap to 1.0.0 #68, the issue body, ADR/CHANGELOG) if this change reshaped it

🤖 Generated with Claude Code

https://claude.ai/code/session_018mujm1kW7jDGpTEfscndcU

BorisTyshkevich and others added 2 commits July 27, 2026 08:26
…ing targets

Phase 2 (absorbs #472). A Dashboard row's name opens View immediately and
Shift-click opens Edit; the chevron became a real disclosure button that only
expands; the trailing control does neither. The delayed single/double
arbitration #426 needed on Dashboard rows is gone — a panel row still
arbitrates, its gestures unchanged.

- model: Dashboard row emits `single: open-dashboard(view)`, `double: null`,
  keeps `shift`. `double === null` is now the view's arbitration switch.
- the `…` menu drops *Open in View* (it IS the primary press) and keeps
  *Open in Edit*, whose only forms are the hidden Shift modifiers.
- chevron: `<button>` with `aria-expanded`, an Expand/Collapse accessible name,
  Enter/Space that stops the key reaching the tree's own Enter, a focus ring in
  a different channel from the row's, and a roving tabindex so the tree stays
  one composite tab stop.
- a search-forced row still cannot be collapsed, but its name now opens it.
- removed two guards the split made unreachable: the chevron's and the #428
  drop handler's `cancelFor`, plus `toggleRow`'s Dashboard arm.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018mujm1kW7jDGpTEfscndcU
Caught in a real browser: hit-testing and getBoundingClientRect both use the
TRANSFORMED box. The new button is 10×24 (stretched to the row height so a 10px
glyph is worth aiming at), so `rotate(-90deg)` on a collapsed row turned its
clickable band into 24 wide × 10 tall — spilling 7px each side and swallowing
clicks meant for the row icon beside it, which then expanded instead of opening.
The schema tree's chevron survives rotating its own box only because it is a
10×10 square.

The rotation now lives on the glyph inside the control, the CSS transition
follows it, and an e2e case asserts the control's box stays inside its own slot
and never reaches the icon — verified to fail with the rotation put back on the
button. happy-dom has no layout, so only e2e can hold this.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018mujm1kW7jDGpTEfscndcU
…keep theirs

A `treeitem` names itself from its CONTENTS, and since the gesture split those
contents include a labelled button — so the chevron's name was being folded into
the row's. Measured in Chromium (Accessibility.getPartialAXTree), not assumed:

  collapsed  "Expand Sales revenue Sales revenue 2"
  expanded   "Collapse Sales revenue Sales revenue 2 Actions for Sales revenue"

That is the opposite of #472's "three independent targets, each ... separately
announced", and it is a regression this phase introduced: the `…` leak was
pre-existing and hover-scoped, but every Dashboard and group row now carried a
permanent "Expand …" prefix.

The row states its own name, composed from the MODEL's strings in the order it
paints them — name, count, status word, meta, marker label — so it cannot drift
from the screen and nothing previously announced is lost. Asserted in the unit
suite (exact names for dashboard/group/orphan-variable/broken-panel rows) and
through `getByRole` in e2e, which resolves real accessible names in every engine;
happy-dom cannot compute one at all. Sabotage-checked: dropping the attribute
fails both.

Also from the review pass: cover the fourth corner of #472's matrix (expanded AND
current), assert a real dblclick dispatches one repeated command, and correct a
CSS comment that called a three-class selector equal-specificity.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018mujm1kW7jDGpTEfscndcU
@BorisTyshkevich
BorisTyshkevich merged commit f82680b into main Jul 27, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant