feat(dashboard): route-history Back/Forward arrows and keyboard chords - #9550
feat(dashboard): route-history Back/Forward arrows and keyboard chords#9550peterhieuvu wants to merge 1 commit into
Conversation
|
Intent: Give the dashboard a first-class Back/Forward over its own route history — visible top-left arrow buttons plus ⌘/Ctrl+←/→ — working identically in the browser and the Electron renderer, with disabled states that never promise a move that will not happen, and every step routed through the draft-discard guard so no pop silently destroys typed work. |
UX Review (Fable 5, fork) — 🟡 CONCERNSUX-level review of All evidence is in hand: the patch is fully read, the four added screenshots are confirmed absent from the base tree (fork lane — binary markers only, none openable), and sibling "Back" affordances checked ( UX-Verdict: CONCERNS Solid browser-convention Back/Forward, but every screenshot lives only on the fork — no first-time reader has seen a single state of it. Watch
Evidence gaps
[UX-REVIEWED] 5fc3a95 |
Design Review (Fable 5, fork) — ✅ PASSDesign-level review of Design review complete. The patch is a self-contained frontend feature (Back/Forward arrows + chords over react-router history), the Design-Verdict: PASS A real gap (#8258) closed at the right seams — one shared position store, the existing leave-guard contract honored, failure modes degrading conservatively rather than lying. [DESIGN-REVIEWED] 5fc3a95 |
First Principles Review (Fable 5, fork) — ✅ PASSPremise-level review of No network access from this environment, so #8258's content stays unverified — that becomes the one human check on an otherwise clean pass. Everything else checked out against the base tree: the moved helpers ( First-Principles-Verdict: PASS Confirm #8258 actually requests dashboard Back/Forward — it is the provenance for the whole addition and this environment could not fetch it. What this change shipsInventory (10 items) — 10 justifiedIntent: give dashboard users a way to retrace their navigation — visible arrows plus keyboard chords — where the desktop app has no browser chrome (#8258). This is an ADDITION.
[FIRST-PRINCIPLES-REVIEWED] 5fc3a95 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed Review detailsThe candidate's premise is that in embed mode the arrows mount (via No new grounded defects at the 80+ bar in the changed lines. No findings. [OPUS-REVIEWED] 5fc3a95 |
GPT 5.6 Review (fork) — ✅ no blocking findings (all downgraded on adjudication)Reviewed Opus 4.8 adjudication downgraded every blocking finding to advisory. Adjudication downgraded all 1 blocking finding(s) to advisory: the remedy each one requires is disproportionate to the harm it prevents. Read them as advice, not as merge conditions. Review detailsBLOCKING -- docs/feature-map/README.md:90 -- Forward can leave the dashboard after a cross-document Back return
Adjudication (Opus 4.8) — is blocking on each finding proportionate?I've read the prompt, the finding, and the full diff. Let me verify the mechanism the finding describes against the code it names. The finding (F1, Tracing it:
So the claim is real, and the module's "one dead click" note understates it for the mid-stack case. Weighing: the consequence is a navigation to a document the user already visited in this same tab, fully recoverable by Back, visible when it happens, and self-correcting on the next in-app PUSH ( The real fix ("disable Forward on cross-document LOW rung; conditions confirmed at [ADJUDICATION] 5fc3a95 total=1 uphold=0 downgrade=1 |
f80dc6e to
7312061
Compare
7312061 to
c37e082
Compare
|
|
|
|
|
|
|
|
Non-ledger notes on the remaining round-1 review items (no span ids — these were not findings):
|
|
CI note for a maintainer: the two reds on head c37e082 (Backend Tests Windows shard 4, and the E2E job's backend phase) are the same single failure — |
|
|
|
|
|
39f4249 to
afb591d
Compare
|
|
|
afb591d to
673fc2f
Compare
|
|
|
|
|
673fc2f to
c445081
Compare
|
|
|
|
Browser-style Back/Forward over the SPA's route history (kirodotdev#8258): two arrow buttons in the header's left cluster (desktop layout), plus Cmd+Left/Right on macOS and Ctrl+Left/Right elsewhere, both driving navigate(+-1) so the NavigationBackGuard draft trap covers them identically. Disabled states derive from react-router's history.state.idx with a conservative Forward watermark (module store, useSyncExternalStore). Chords are gated out of text fields and terminals to preserve caret and PTY meanings. i18n keys in all 13 catalogs + en-XA + translator context.
c445081 to
5fc3a95
Compare
Problem / Motivation
Moving around the dashboard is a one-way street. Chat → Settings → an app page leaves no way back to where you came from: the desktop app has no browser chrome, no keyboard shortcut, and no visible control walks the route history. Some pages hand-roll a "Back" link, but it points at a fixed parent, not at where you actually were (#7884 is that gap on app pages).
Why it matters
Retracing your steps is a reflex every browser and most desktop apps honor. Without it, each wrong turn in the dashboard costs a manual re-navigation, and on desktop — where users expect OS conventions — there is no recovery gesture at all. #8258 asks for exactly this.
What changed (motivation → approach → change)
The header's top-left now has Back and Forward arrow buttons, and
⌘←/⌘→(macOS) orCtrl+←/Ctrl+→(Windows/Linux) do the same thing from the keyboard. Both drive react-router's own history (navigate(±1)), so they work identically in the browser dashboard and the Electron renderer, and desktop session switches — which push real history entries — retrace along with pages.Each arrow knows when it has nowhere to go. A new module store (
src/lib/routeHistoryPosition.ts) reads react-router'shistory.state.idxthrough the samerouterEntry()readerNavigationBackGuarduses: Back exists iffidx > 0(exact, survives reloads), Forward iffidxis below a watermark of the highest index seen, which a PUSH resets because a push truncates the Forward branch. The watermark is kept insessionStorage(per tab, dies with the tab — the stack's own lifetime), so a Forward branch built before a reload or a Back/Forward return is still offered afterwards. A freshnavigatearrival (typed URL,location.href, a popout that cloned the tab's storage) truncates that branch, so there the stored value is discarded and the arrow under-reports rather than promising a move that will not happen. ARouteHistoryTrackermounted next toNavigationBackGuardfeeds the store; the arrows and the chord read it. On a very narrow header (below a 208px left-cluster width, the documented drop-rung pattern) the arrows hide — they are the one redundant control in that cluster, since the chords and browser chrome reach the same history.Every step asks before it destroys. A pop is the one navigation the draft-discard trap (
NavigationBackGuard) already guards — but only while its trap is armed, and the trap deliberately stays out of the stack when the user has a Forward branch or after a reload. The newuseGuardedHistoryStep()closes that gap: it asks the page's leave guard up front unless the current entry is an armed trap duplicate, in which case the trap prompts instead. One prompt per click, wherever the ask lives. The chords are additionally gated out of text fields (where the same keys mean caret line-start/line-end or word-jump), terminals (the PTY owns them), and narrow viewports — the same condition that hides the arrows, because below the breakpoint the drill-in surfaces navigate by component state and a stack walk would move history the visible UI does not reflect.Entering the dashboard through a
?token=link used to leave the arrows permanently disabled: ChatPage's two URL strippers (the?prefill=consumer and the channel-token consumer) calledhistory.replaceState({}, …), erasing react-router'shistory.state.idxbookkeeping so every later index was NaN. Both now passwindow.history.statethrough — the query is still stripped, the router's state survives — androuterEntry()readsidxwithNumber.isFinite, so any future raw replace degrades the arrows to conservatively-disabled instead of poisoning every comparison.The arrows advertise their chords only while they work:
aria-keyshortcutsand the tooltip suffix ("Back (⌘←)") read the same shortcuts-enabled toggle the keydown handler honours, through the shareduseShortcutsEnabled(), and read the LIVE binding (useShortcutBindings()) rather than the factory default, so a rebound chord is announced correctly and an unbound one not at all; the catalog tooltip can only spell the factory chord, so it drops to the bare label once the user rebinds.The chords are
dispatch: 'registry'entries in the shortcut registry (history-back/history-forward,actionsgroup), so the #4608 rebind UI covers them for free; a text field un-claims the hit (the field consumes the caret chord), while a narrow viewport — where the arrows are hidden — claims the chord as a deliberate no-op, because on macOS an unclaimed⌘←is native browser Back and would pop past an unarmed draft trap.⌘[/⌘]stays on session cycling; the bracket-convention question from #8258 remains a maintainer call this PR does not preempt.Tests
NavHistoryArrows.test.tsx— arrows over a realBrowserRouter: disabled on a fresh document, Back enables after a push, walking back enables Forward, a mid-stack push truncates the Forward branch,aria-keyshortcutson both buttons; store unit tests (snapshot identity stability, subscriber notification only on change, null-idx claims nothing, livecanGoBackvs watermark-dependentcanGoForward).useKeyboardShortcuts.test.tsx—historyNavStepplatform/modifier discipline (⌘ vs Ctrl, rejects Shift/Alt/extra modifiers, vertical arrows); handler integration: navigates when history allows, claims the chord at the stack bottom so the arrows and keyboard agree, leaves the chord to text fields and terminals.Manual verification
Verified live against a dev gateway with Playwright: fresh load shows both arrows disabled; navigating enables Back; the Back arrow pops and enables Forward; Forward returns;
Ctrl+←navigates back with the same state flips. Screenshots below are from that session.Additionally verified by hand on the macOS desktop app against a dev gateway (via a remote-instance connection): arrows render in the header's left cluster clear of the traffic lights, enable/disable correctly across navigation, and the token-entry regression fixed on this head stays fixed on that path.
Screenshots / video
Fresh load — both arrows disabled (nowhere to go):
After navigating — Back enabled, Forward disabled:
After stepping Back — Forward enabled, Back disabled (dark theme):
The two new shortcuts-modal rows under Actions:
Related Issues
Fixes #8258
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)