feat(#487): rail, focused drawer and resize separator (phase 3) - #574
Open
BorisTyshkevich wants to merge 12 commits into
Open
feat(#487): rail, focused drawer and resize separator (phase 3)#574BorisTyshkevich wants to merge 12 commits into
BorisTyshkevich wants to merge 12 commits into
Conversation
…step 1) Adds the pure core pieces phase 3 needs before any UI wiring: a resize session (begin/advance/commit) that fixes the sampling-dependent restore memory from phase 1 for both bands without reintroducing it (two rounds of adversarial review caught that "continuously updated" memory has the same bug relocated), and clampLeftNavigationToMaximumTotal + the viewport- aware separator ARIA ceiling for the centre-width safety constraint. No production consumer yet — wiring lands in later steps of this phase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
New src/application/left-nav.ts: openFocusedSection/toggleFocusedSection compose the pane-selection write and the layout write into ONE batch(), fixing a real atomicity gap the phase-2 handoff flagged (two independently batched functions can let an effect observe a mismatched intermediate state). Also fixes a confirmed persistence gap: opening a lower section via this seam now persists `sidePanel` exactly like the wide sidebar's own tab switch already does (saved-history.ts's switchTo), so a rail/drawer selection survives a reload. Wires app.openFocusedSection as the deterministic seam #428 needs. No UI calls it yet — the rail/drawer land in a later step of this phase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
…phase 3, step 3) Replaces the shared state.libraryFilter with lowerNavigationFilters, a per-section record — switching between Library and History no longer clears the search box, required by this phase's own acceptance bullet that wide and focused presentations share and preserve all navigation state (a deliberate, user-visible change from phase 2's "unchanged" gate, documented in the CHANGELOG). Splits renderSavedHistory so both lower sections render their own content unconditionally, mirroring the upper pane's renderSchema/ renderDashboardTree pattern, closing two confirmed bugs: a section not active at mount never painted until the first switch, and a section's content going stale when its own data changed while the other section was exposed (History-while-Library-active, and the reverse). The two call sites that guarded a History repaint behind `sidePanel === 'history'` (app.recordHistory, the script-run history path) are now unconditional. Also closes #572: the lower switcher's tabs gain type="button" and aria-pressed, matching the upper switcher. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
An independent ChatGPT review of the phase-3 steps 1-3 diff found a real bug, confirmed by hand: commitLeftNavigationResize compared two POST-CLAMP layouts (effective vs effectiveAtStart) to decide whether a band's width changed. That's wrong for a restore command (Home/End/a bare-rail ArrowRight) under an active viewport clamp — a bare rail's dormant preferred width passes through effectiveAtStart unclamped (there is nothing to clamp yet), so a restore's honest, unclamped proposal gets compared against it, differs, and the TRANSIENT clamped render value gets committed instead of the user's real preference. E.g. a 420px preference restored on a ~800px window would silently downgrade to ~313px. Fix: the session now tracks the RAW reducer proposal separately from the clamped effective layout, and the commit decision compares the raw proposal against the preference, never the clamped value. This also folds the viewport clamp into advanceLeftNavigationResize itself, so a future caller can no longer forget to apply it. Also: openFocusedSection/toggleFocusedSection no longer re-persist sidePanel when the value is already current (relevant since #428's bounded drag-hover reasserts the same section repeatedly), a stale test that checked a field name already renamed in step 3, a dead field on WorkbenchStateSlice, and stale doc references in ADR-0001 and saved-query-service.ts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
Two new standalone modules, not yet wired into the app shell: - src/ui/left-rail.ts: the compact icon rail, four launchers built from the section registry (nav-sections.ts) so a rail tooltip/aria-label can never disagree with the wide switchers' own label for the same section. A click routes through toggleFocusedSection. - src/ui/left-nav-separator.ts: the resize/mode-changing separator that will replace splitters.ts's 'col' axis in the next step. Mirrors splitters.ts's existing mouse-event drag model (not Pointer Events). Every pixel decision routes through the LeftNavigationResizeSession from left-nav-layout.ts; this module's own job is pointer/keyboard mechanics, session bookkeeping, and ARIA, never the resize arithmetic or painting the sidebar directly (that's the injected applyEffectiveLayout seam a later step implements). Terminates safely on blur and visibilitychange; mouseup processes its own final coordinate rather than the last mousemove's. Caught during review before wiring anything up: commitSession never called applyEffectiveLayout, so a keyboard-driven resize updated state and ARIA but never actually repainted the sidebar (the pointer path only worked because advanceTo already paints on every mousemove/final mouseup). Fixed by having commitSession paint with the session's own final effective layout — the one place a keyboard resize ever reaches the DOM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
…-shell (phase 3, step 4b) The feature becomes reachable. .sidebar is RE-PRESENTED, never moved or duplicated: a data-nav-mode attribute (wide/rail/drawer) selects between the wide two-pane sidebar and the rail's focused drawer, written by ONE function (applyEffectiveLeftNavigationLayout) that is the sole authority for every presentation attribute — hidden toggles, width, the drawer's title, aria-labelledby. It's called by initial mount, a "preferred-state" effect (mode/section/isMobile changes), the resize separator's own session steps, and an injected shell-width observer seam (so a plain browser-window resize with no active drag still honors the centre-content minimum). splitters.ts's 'col' axis is deleted entirely — left-nav-separator.ts owns that gesture now, routing every proposal through the mode reducer instead of a bare clamp. Reconciles four stale comments (nav-sections.ts, styles.css) that described DOM host movement, a design this phase rejected in favor of re-presentation. Also wires a real ResizeObserver-backed default for the shell-width observer seam into app.ts (mirroring the existing matchMedia injection pattern) — the seam existed and was tested from the prior commit but had no production implementation, so a real user's window resize would not have triggered it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
…licts A second ChatGPT review of the rail/separator/app-shell composition found four real bugs, all confirmed by hand before fixing: 1. CRITICAL: .side-pane/.side-tabs have `display: flex` class rules with no `[hidden]` override, so setting `.hidden` on the wide-only panes and tab rows did NOTHING in a real browser — the exact footgun this codebase already guards against for .query-host/.dashboard-host/ .nav-section-host/.left-rail/.left-nav-title. Every unit test passed because happy-dom never computes CSS, but a focused drawer would have shown both panes and both tab switchers simultaneously in production. 2. The separator's own ARIA (aria-valuenow/valuemax/valuetext) went stale whenever the width changed from outside its own gesture (mount, a plain window resize) — its internal effect only re-runs on a mode/ section signal change, never a width-only one. Fixed with a new refreshAria() the app-shell's presentation function calls on every one of its own invocations. 3. The ResizeObserver-driven reclamp could silently overwrite an active drag's in-progress (not-yet-committed) paint with stale committed state, visibly snapping the sidebar away from the pointer mid-drag. Fixed with a new isSessionActive() the observer callback checks before repainting. 4. A keyboard press could commit a second, conflicting session while a pointer drag was still active (reachable: Tab-focus the separator, mousedown it too, then press an arrow while the button is held). Fixed by ignoring keyboard input while a pointer session is active. Plus small corrections: a stale CHANGELOG note claiming ResizeObserver wiring was still a follow-up (it already landed), a stale "standalone, not wired yet" module comment, an inaccurate per-button-selectivity comment, and dispose() now clears the .dragging class and any in-progress session instead of leaving them dangling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
…e 3, step 5) Escape closes a focused drawer and returns focus to the rail launcher that opened it, without acting when a nested handler already claimed the key (a non-empty search filter, a saved-row edit form's own Escape-to-cancel) or when there's no drawer to close. Fixes a real bug along the way: the search box's Escape handler claimed every Escape unconditionally, even when already empty, so Escape could never bubble up to close the drawer while focus sat in an empty search box. Converting a focused drawer to the wide sidebar (a resize-separator drag past the wide threshold, or the End/bare-rail-ArrowRight keyboard restore) restores focus to that section's own wide-mode tab, resolved by a live querySelector at the moment of the transition since the tab rows rebuild on every repaint. Both wide switchers' tab buttons gain a data-section attribute to make this (and the rail's own focus-restore target) addressable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
…to 180px Real-browser (Playwright, chromium+webkit) coverage for the feature happy-dom cannot see: folding to the rail, opening/switching/closing a focused drawer, the drawer pushing the work surface rather than floating over it, Escape-to-close with focus restoration, dragging back to wide with focus landing on the matching tab, separator keyboard operation, and the mobile rail-suppression fallback. Re-fixtures the one e2e test that was actually stale under the new fold/hysteresis rule (a drag to clientX=100 used to clamp to a 180px floor; it now folds to the rail) into two tests — the dead zone at a different coordinate, and the new fold itself. This pass settled phase 1's open question about the focused drawer's resize floor: at 140px (as shipped) the Dashboards section's three titles rendered as "Sa...", "O...", "A..." — unreadable and indistinguishable — while at 180px they read as "Sales re...", "Ops late...", "A very lo..." — still ellipsized but genuinely readable. Raised the drawer's floor to 180px, reusing LEFT_PANEL_MIN_PX rather than a second constant, which gives the drawer the wide sidebar's own dead-zone mechanism for free from the existing clamp — except the keyboard path, which needed an explicit mirror of the wide sidebar's own "already at floor, step left folds" check, since arrow keys take fixed relative steps that can get stranded in the newly-separated fold-threshold/floor gap the way the wide sidebar's keyboard handling already had to solve once before. Also fixes a real, confirmed WebKit-only bug the e2e pass surfaced: clicking a rail icon to close its own drawer didn't return focus to the icon on Safari (WebKit doesn't focus a clicked button natively, unlike Chromium/Firefox, unlike the Escape path's explicit focus call). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
…ch panes Independent review found a real regression: revealDashboard (File Menu -> New Dashboard, and after an import commits) and revealAssignedPanel (Library "Add to dashboard...", drag/drop settlement, Panels-row create-and-assign) wrote state.upperRole directly, bypassing the openFocusedSection seam this phase built specifically so opening a section works whether the nav is wide, folded, or already showing a drawer on something else. Before this phase the sidebar was always the wide two-pane view, so a bare upperRole write was always enough to make the Dashboards pane visible. Now that folding exists, folding the nav (or opening a drawer on Library/History/Databases) before triggering either reveal action left the tree's own expand/scroll/select work happening inside a hidden pane, or, at a bare rail, behind an entirely hidden sidebar. Both now route through openFocusedSection, which reduces to the exact same upperRole write in wide mode (resolveRailOpen is a no-op there) and additionally opens/switches the focused drawer when folded. Also: closes #572 (never logged in the CHANGELOG when it shipped in an earlier commit of this phase), and a stale nav-sections.ts comment still describing host movement between containers instead of the re-presentation this phase actually uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
This was referenced Jul 30, 2026
Open
A comprehensive final review of PR #574's complete diff found four real, confirmed bugs interacting across pieces that were each reviewed separately earlier: leftNavSection going stale across a mobile round-trip, an active resize session persisting through that same breakpoint crossing, a blur/visibilitychange commit using a stale pre-shrink clamp, and focus restoration firing on transient frames of a non-monotone drag. All four verified against the real code before fixing, and fixed together since two share one reconciliation point (the isMobile-transition handling). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZfeGpkpJw2zsfPtysGHUN
A fresh integrated pass on PR #574 found one new preference-corruption blocker and two remaining interaction/accessibility gaps, all confirmed by hand before fixing: 1. BLOCKER: a click-and-release on the resize separator with no genuine movement could silently overwrite the stored wide/drawer width preference — mousedown recorded no starting coordinate or grip offset, so mouseup always treated the release clientX as a brand-new raw proposal, even a pure click landing wherever a viewport clamp happened to render the handle. The session now records the grip point and skips advancing entirely on a genuine no-op click; every advance also subtracts the recorded grip offset so a drag grabbed anywhere in the 7px handle tracks the pointer from its own grab point rather than the handle's left edge. 2. A semantic left-navigation command (Escape, a rail click, a programmatic reveal) running while a pointer resize was still in progress could be silently undone by that drag's own eventual commit, since the drag keeps its own uncommitted layout snapshot independent of state. application/left-nav.ts's openFocusedSection/toggleFocusedSection now call an injected preemptActiveResize seam first, which app-shell.ts wires to cancel the active session and repaint from the committed layout — one choke point every caller goes through, rather than each call site having to remember it. 3. Crossing into the mobile breakpoint from an open focused drawer could move focus onto a hidden desktop tab button, because the wide-mode focus restoration keyed only on navMode === 'wide', which the mobile projection also forces. Now gated behind !state.isMobile.value. 4. Folding an open drawer to the bare rail via a pointer drag left focus stranded in the drawer's now-hidden content, with no restoration to the rail launcher — the fourth transition, unlike Escape-close/rail-icon- close/drawer-to-wide, that had none. Scoped to sidebar.contains(document.activeElement) so a keyboard fold (which needs the separator itself focused) is left alone. Also corrected two CHANGELOG overstatements (live-region announcement coverage; an "omitted in every test" claim contradicted by several tests that inject observeElementWidth) and added a forward-looking caveat on the 480px centre-minimum "never" claim pending phase 4's right inspector. Every fix has unit coverage (including a direct pure-seam test for preemptActiveResize) and, for the click-without-drag blocker, real-browser Playwright coverage across chromium/webkit. Confirmed each new/updated test actually fails without its corresponding fix before restoring it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgVCic37D9gREMNZDUk9BN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Part of #487 (phase 3 of 4) — the feature becomes reachable. The desktop left navigation can now be folded to a 48px icon rail by dragging its border, and a rail icon opens that section (Databases/Dashboards/Library/History) as a docked, full-height focused drawer that pushes the work surface rather than floating over it.
Builds on phase 1 (
core/left-nav-layout.ts— thresholds, mode reducer, keyboard resolver, ARIA range, #571) and phase 2 (the navigation section registry giving all four sections one persistent host each, #573).What shipped
core/left-nav-layout.ts) fixing a sampling-dependent restore-memory bug phase 1 flagged as an open question, without reintroducing it in a different shape — two independent design/code reviews caught and fixed two increasingly subtle versions of the same underlying mistake before this landed.application/left-nav.ts) —openFocusedSection/toggleFocusedSectionatomically batch the wide-mode pane switch and the fold/drawer state together, the deterministic seam Add Library drag assignment to Dashboard panels and variables #428's drag-hover integration needs.saved-history.ts) — Library and History now render independently of which is exposed (mirroring how the upper Databases/Dashboards pane already works), fixing two latent bugs (a section not active at mount never painted; a section's content could go stale while the other was exposed) and closing Lower sidebar switcher announces no pressed state (upper pane does) #572 along the way.left-rail.ts,left-nav-separator.ts) — new, standalone, fully-seamed modules; the separator replacessplitters.ts's old bare-clamp'col'axis outright..sidebarre-presented, never moved — the existing sidebar element becomes the drawer via adata-nav-modeattribute; no DOM host ever moves between containers, so every section's search/expansion/scroll state survives a mode change by construction.[hidden]on the drawer's non-focused pane/tab-rows did nothing in an actual browser, because the underlying CSS classes haddisplay: flexwith no override. Fixed, and now covered via computed style, not just the DOM property.Full detail — every decision, deviation, and review finding — is in the ship-log comment on #487.
Acceptance criteria claimed
Bullets 1–6 and 8 of #487's
## Acceptance criteria. Bullet 7 (header/main-surface/right-inspector reflow) is phase 4's.Deviations from the issue's suggested code boundaries
The issue's phase-3 section names a new
left-drawer.tsmodule and asplitters.tsaxis addition. Neither happened: the drawer is.sidebarre-presented (no separate module owns its markup — this is what makes state-preservation-by-construction true), and the separator is its own dedicated module replacing'col'outright rather than adding a fifth competing axis. Both are deliberate; rationale is in the code's own design comments and the ship log.Checklist
npm testpasses (the per-file coverage gate is non-negotiable)npm run buildsucceeds (single-filedist/sql.html)src/core/, DOM insrc/ui/behind injected seams (ResizeObserver, mouse/keyboard events)CHANGELOG.md([Unreleased]) updated — README not touched: it has nosection documenting the sidebar's fold/rail/drawer behavior yet, and
that's deferred to phase 4, once the header/main-surface/right-inspector
reflow completes the whole story
Closes #572.