Skip to content

feat(#487): left-navigation layout core and preferences (phase 1) - #571

Merged
BorisTyshkevich merged 2 commits into
mainfrom
feat/left-nav-layout-core-487p1
Jul 29, 2026
Merged

feat(#487): left-navigation layout core and preferences (phase 1)#571
BorisTyshkevich merged 2 commits into
mainfrom
feat/left-nav-layout-core-487p1

Conversation

@BorisTyshkevich

Copy link
Copy Markdown
Collaborator

What & why

Phase 1 of 4 for #487 — a drag-foldable desktop left navigation with a rail and focused
drawers. This phase adds the pure module that owns every layout decision the feature needs,
plus the two browser preferences behind it. No user-visible change: the rail, the docked
focused drawer and the role="separator" resize control arrive in phase 3.

src/core/left-nav-layout.ts owns the named constants and thresholds, the explicit
'wide' | 'rail' mode, the hysteresis reducer, the keyboard separator resolver, rail
activation, the separator's ARIA range, and the mobile projection.

Hysteresis is the threshold pair — folding needs a proposal below 140px, restoring one
above 260px — so the mode is sticky between them and no single pointer pixel can oscillate it.
The keyboard path routes through the same reducer as the pointer path, so the issue's "keyboard
separator operations match pointer transitions" holds by construction rather than by two
implementations agreeing.

Phase structure and per-phase scope are recorded in the issue body; the <!-- ship-log -->
comment is the state of record.

One bug worth calling out

Every function takes the navigation's proposed total width and derives each mode's own
panel width itself. An earlier revision handed the reducer a mode-relative width, and review
caught that this made a monotone rightward drag snap the navigation 108px backwards on the
frame a drawer converted to the wide sidebar — the two measurements disagree by exactly the
rail's width at the crossing:

clientX 308 -> rail, occupied 308
clientX 309 -> wide, occupied 200   <- backwards
clientX 310 -> wide, occupied 310

Every per-frame assertion had passed, because each frame's output was individually defensible;
only the sequence was wrong. The regression tests now sweep a whole pointer path and assert the
width response is monotone in both directions.

Deliberate deviations from the issue's suggested shape

  • The wide width reuses the existing asb:sidebarPx rather than adding a parallel
    wideWidthPx. That key already persists exactly this width over exactly this range, and two
    owners of one width is a bug waiting to happen. New keys are only asb:leftNavMode and
    asb:leftNavDrawerPx.
  • The focused drawer's band is [fold, wide] = [140, 260], not the wide sidebar's
    [180, 420]. A drawer drag must fold below the fold threshold and convert above the wide
    one, so most of the wide range is unreachable for a drawer; MIN/MAX govern the sidebar.
  • A drag follows the pointer and does not restore remembered widths. The issue asks a
    rail → wide drag for both "restore the last useful wide width" and "deterministic resize
    feedback", and those cannot both hold — the next pointermove overwrites whatever the crossing
    frame installed, so a restored width survives one frame and reads as a flicker. End
    restores the remembered width; it is the discrete counterpart, with no pointer to honour.
  • The centre-width clamp moved to phase 4, where its caller is. A single
    total-in/total-out signature is the wrong shape — there is no inverse from a total back to
    (mode, panel width), so it could return a width no mode can render (220px is neither the
    rail, nor a legal wide sidebar, nor rail+drawer). Deferring also respects hard rule 5.

The issue body has been reconciled for all four.

Two things found on the way

  • splitters.ts's 'col' axis kept its own copy of [180, 420] as literals while the
    load path moved onto the constants — and that axis is what writes sidebarPx. Now clamps
    through clampWideWidthPx. Behaviour unchanged (a real drag always carries a finite
    clientX); the sidebar drag e2e specs pass untouched.
  • 'library' is this module's section name but not the value AppState.sidePanel stores.
    That is still 'saved'Separate Library queries from dashboard-owned query copies #427 renamed the label and deliberately left the persisted value
    alone. Phase 2's registry owns the single mapping; the type's doc records it so phase 2 does
    not rediscover it the hard way.

Filed separately

#570 (inbox) — clamp is not NaN-safe (Math.max(180, NaN) is NaN), so
editorPct/sideSplitPct/cellDrawerPx/docPanePx all decode a corrupt stored value to
NaN, reaching the DOM as width: NaNpx which the browser drops. sidebarPx is fixed here
because this phase owns it; the other four are out of scope.

Sabotage-checked

Each of these fails a specific test: reverting the crossing to the remembered width, basing the
bare-rail arrow step on wideWidthPx (which would strand a bare rail's small ArrowRight in
the sticky band forever for any remembered width ≤ 244), loosening the drawer's fold comparison
to <=, and adding a field to the workspace projection.

Checklist

Part of #487

🤖 Generated with Claude Code

https://claude.ai/code/session_01FPnt3pq98P5Y1ww3sWawp1

Phase 1 of 4 for the drag-foldable desktop left navigation. Adds the pure
module that owns every layout decision the feature needs, plus the browser
preferences behind it. No user-visible change: the rail, the docked focused
drawer and the resize separator arrive in phase 3.

`src/core/left-nav-layout.ts` holds the named constants and thresholds, the
explicit 'wide' | 'rail' mode, the hysteresis reducer, the keyboard separator
resolver, rail activation, the separator's ARIA range, and the mobile
projection. Hysteresis is the threshold PAIR — folding needs a proposal below
140px, restoring one above 260px — so the mode is sticky between them and no
single pointer pixel can oscillate it. The keyboard path routes through the
same reducer as the pointer path, so "keyboard operations match pointer
transitions" holds by construction rather than by two implementations agreeing.

Every function takes the navigation's proposed TOTAL width and derives each
mode's own panel width itself. An earlier revision handed the reducer a
mode-relative width, and that made a monotone rightward drag snap the
navigation 108px BACKWARDS on the frame a drawer converted to the wide sidebar,
because the two measurements disagree by exactly the rail's width at the
crossing. The regression tests now sweep a whole pointer path and assert the
width response is monotone; per-frame assertions could not see it, since each
frame's output was individually defensible.

Deviations from the issue's *suggested* shape, all deliberate:

- the wide width reuses the existing `asb:sidebarPx` rather than adding a
  parallel `wideWidthPx`. That key already persists exactly this width over
  exactly this range; two owners of one width is a bug waiting to happen. The
  new keys are only `asb:leftNavMode` and `asb:leftNavDrawerPx`.
- the focused drawer's band is [fold, wide] = [140, 260], not the wide
  sidebar's [180, 420]. A drawer drag must fold below the fold threshold and
  convert above the wide one, so the wide range is mostly unreachable for a
  drawer; MIN/MAX govern the sidebar.
- a drag follows the pointer and does NOT restore remembered widths. The issue
  asks a rail -> wide drag for both "restore the last useful wide width" and
  "deterministic resize feedback", and those cannot both hold: the next
  pointermove overwrites whatever the crossing frame installed, so a restored
  width survives one frame and reads as a flicker. `End` restores the
  remembered width — it is the discrete counterpart, with no pointer to honour.
- the centre-width clamp moves to phase 4, where its caller is. A single
  total-in/total-out signature is the wrong shape: there is no inverse from a
  total back to (mode, panel width), so it could return a width no mode can
  render. Deferring also respects CLAUDE.md hard rule 5.

Also drops the second owner of [180, 420]: `splitters.ts`'s 'col' axis, which
WRITES `sidebarPx`, kept its own copy of the bounds as literals while the load
path moved onto the constants. Behaviour is unchanged (a real drag always
carries a finite clientX); the sidebar e2e drag specs pass untouched.

`clampWideWidthPx` closes a NaN hole on the way past: `clamp` is not NaN-safe
(`Math.max(180, NaN)` is NaN), so a corrupt `asb:sidebarPx` would have decoded
to `width: NaNpx`, which the browser drops. Hardening, not a reproducible bug —
no code path writes a non-numeric value. The same hole in the four sibling
geometry keys is filed as #570 (`inbox`).

`'library'` is this module's section name but NOT the value `AppState.sidePanel`
stores for it: that is still `'saved'`, because #427 renamed the label and left
the persisted value alone. Phase 2's registry owns the one mapping; the type's
doc records it so phase 2 does not rediscover it the hard way.

npm test 6734 passing, `left-nav-layout.ts` and `splitters.ts` both at
100/100/100/100. Sabotage-checked: reverting the crossing to the remembered
width, basing the bare-rail arrow step on `wideWidthPx`, loosening the drawer's
fold comparison to `<=`, and adding a field to the workspace projection each
fail a specific test.

Part of #487

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FPnt3pq98P5Y1ww3sWawp1
A third-party review pass raised six findings against the phase-1 module. All six
reproduced against the real code; all six are fixed here. The most valuable one is
the exact mirror of the bug the earlier review caught, which this suite had been
blessing with a test.

1. **Plain ArrowLeft could never fold a wide sidebar.** At the 180px floor a -16
   step proposes 164, which clamps straight back to 180 — so eleven presses from a
   300px sidebar sat at 180 forever while the equivalent pointer path folded:

     keyboard  284,268,252,236,220,204,188,180,180,180,180 -> wide
     pointer   284,268,252,236,220,204,188,180,180,180,48  -> rail

   This is the same class of dead end as the bare-rail one already fixed, at the
   opposite end, and the previous commit's test explicitly blessed it on the
   grounds that Home and Shift+Arrow escape. That is not a defence: the W3C
   splitter pattern makes plain Left/Right the separator's move keys, and
   `aria-valuemin: 48` was advertised while the control refused to move.

   Arrows now resize within a band and perform the band edge's semantic
   transition, symmetric at both ends. `keyboardBaseTotalPx`'s virtual 260 base is
   gone with it — it was a false relative step (ArrowRight from a 48px rail moved
   +228, not +16) and it discarded a remembered 420, handing back 276. A bare
   rail's ArrowRight now restores the remembered width, like End.

2. **`resolveRailActivation` is a toggle, so it cannot be the `openFocusedSection`
   seam** #487 mandates for #428. Bounded drag-hover re-asserts intent while a
   query is held over the Dashboards icon, so a toggle flaps the drawer open and
   shut on alternate notifications. Added `resolveRailOpen`, idempotent and
   identity-returning when already open; the toggle stays for clicks.

3. **The coherence invariant was a precondition, not a postcondition.** `state.ts`
   stores `mode` and `focusedSection` as two independently writable signals, and
   the reducers preserved an illegal pair rather than healing it — `drag({mode:
   'wide', focusedSection: 'databases'}, 300)` returned it intact, and `End` handed
   it straight back. Every reducer now normalizes its input through
   `normalizeLeftNavigationLayout`, which also lifts non-finite and out-of-band
   widths, so `leftNavigationLayoutIsCoherent` is "normalizing changes nothing" and
   covers widths rather than only the mode/section pairing. A NaN width can no
   longer reach `aria-valuenow`.

4. **`parseInt` accepted a numeric prefix**, so `'12junk'` decoded to 12 and
   `'200px'` to 200 while the documented contract promised the default. Added
   `decodeStoredPx`, which requires the whole string to be a finite number — and
   therefore also rejects a stored `'Infinity'`, which is corruption rather than a
   width pressed against a bound.

5. **The restore memory is sampling-dependent** and this commit does NOT fix it:
   from a 300px sidebar, a single coarse sample past the fold remembers 300, but an
   intermediate sample inside the 140-179 dead zone rests the width at the floor
   first and remembers 180. One field is serving as both the live drag width and
   the restore memory; separating them needs a drag-session snapshot, which a pure
   reducer cannot take. Pinned with a test so phase 3 has to change it
   deliberately, and recorded as a phase-3 obligation in the ship log.

6. **The centre clamp's phase boundary was unsafe.** Phase 3 turns the feature on
   while the clamp sat in phase 4, leaving a shippable interval where both docked
   panels could starve the centre surface. Moved to phase 3, before activation.

Two findings were verified and NOT changed, with reasons recorded in the ship log:
the drawer's [140, 260] band (a 150px drawer may be unusably narrow, but #487 says
these constants are settled only by real-browser verification), and the separator's
discontinuous ARIA interior (inherent to one control spanning two modes; phase 3
adds mode-aware `aria-valuetext`).

npm test 6755 passing, `left-nav-layout.ts` at 100/100/100/100 (17 functions, 64
lines, 86 branches). Sabotage-checked: dropping the wide-floor transition, making
the open seam a toggle, removing reducer normalization, and reverting to parseInt
each fail specific tests. Sidebar drag e2e green.

Part of #487

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

Copy link
Copy Markdown
Collaborator Author

Third-party review pass (ChatGPT) — six findings, all reproduced, all fixed

Per /ship step 7. ChatGPT returned request changes. I reproduced every claim against the
real code before accepting any of it (scratch spec driving the actual reducers), and all six
were real. Fixed in acc7317.

1. Plain ArrowLeft could never fold a wide sidebar. At the 180px floor a −16 step proposes
164, which clamps back to 180. Eleven presses from a 300px sidebar:

keyboard  284,268,252,236,220,204,188,180,180,180,180 -> wide   (stuck forever)
pointer   284,268,252,236,220,204,188,180,180,180,48  -> rail

This is the exact mirror of the bare-rail dead end the first review round caught and I
fixed — and the first round's test explicitly blessed this one, on the grounds that Home and
Shift+Arrow escape. That isn't a defence: the W3C splitter pattern makes plain Left/Right the
separator's move keys, and aria-valuemin: 48 was advertised while the control refused to move.
Arrows now resize within a band and perform the band edge's semantic transition, symmetric at
both ends. The virtual 260 base is gone with it — it was a false relative step (ArrowRight
from a 48px rail moved +228, not +16) and it discarded a remembered 420, handing back 276.

2. resolveRailActivation is a toggle, so it can't be the openFocusedSection seam #487
mandates for #428. Bounded drag-hover re-asserts intent repeatedly, so a toggle flaps the drawer
open and shut on alternate notifications. Added resolveRailOpen — idempotent, identity-returning
when already open. The toggle stays for clicks.

3. The coherence invariant was a precondition, not a postcondition. state.ts stores mode
and focusedSection as two independently writable signals, and the reducers preserved an
illegal pair rather than healing it — drag({mode:'wide', focusedSection:'databases'}, 300)
returned it intact and End handed it back. Every reducer now normalizes its input, which also
lifts non-finite and out-of-band widths, so a NaN can no longer reach aria-valuenow.

4. parseInt accepted a numeric prefix'12junk' → 12, '200px' → 200 — while the
documented contract promised the default. Added decodeStoredPx, requiring the whole string to
be finite (so a stored 'Infinity' is corruption, not a width against a bound).

5. The restore memory is sampling-dependent — NOT fixed here, deliberately. From a 300px
sidebar, one coarse sample past the fold remembers 300, but an intermediate sample in the 140–179
dead zone rests at the floor first and remembers 180. wideWidthPx is serving as both the live
drag width and the restore memory; separating them needs a drag-session snapshot, which a pure
reducer cannot take. Pinned with a test so phase 3 must change it deliberately, and recorded as a
phase-3 obligation in the ship log. (This does not argue for a second persisted key —
asb:sidebarPx stays the one preference; the extra quantities are per-gesture.)

6. The centre clamp's phase boundary was unsafe. I had moved it to phase 4, but phase 3
turns the feature on
— that would leave a merged interval where left and right docked panels can
starve the centre surface. Moved into phase 3, before activation. Deferring an unused primitive
out of phase 1 is hard rule 5; deferring a safety constraint past the feature it protects is not.
Issue body reconciled.

Verified and deliberately NOT changed

  • The drawer's [140, 260] band. A 150px drawer is legal today and must hold a title, search,
    tree indentation and actions. The real alternative isn't [180, 420] — it's to mirror the wide
    dead zone (fold below 140, hold at a 180 floor through 140–179, resize 180–260, convert above
    260). Add a drag-foldable desktop left navigation with rail and focused drawers #487 says these constants are settled "only through real-browser layout verification", so
    this is a phase 3/4 call at 140/179/180/260, not a unit-test one. Recorded as an open question.
  • The separator's discontinuous ARIA interior. 49–179 is no resting width in any mode, and one
    number can mean either a wide sidebar or rail+drawer. Inherent to one control spanning two
    modes; phase 3 adds mode-aware aria-valuetext (now in the issue body) rather than reshaping
    the range.
  • isLeftNavigationSection as a "speculative export" — it now has a production caller
    (normalizeLeftNavigationLayout validates the section through it).

Gate after the fixes

npm test 6755 passing; left-nav-layout.ts 100/100/100/100 (17 functions, 64 lines, 86
branches); tsc --noEmit, arch and schema checks clean; bundle builds; sidebar drag e2e green.

Sabotage-checked — each fails a specific test: dropping the wide-floor transition, making the open
seam a toggle, removing reducer normalization, reverting decodeStoredPx to parseInt, plus the
four from round one.

@BorisTyshkevich
BorisTyshkevich merged commit 6715b42 into main Jul 29, 2026
8 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