fix(ui): stop the site-wide g-shortcut handler from racing app-shell nav#6959
Conversation
…nav (JSONbored#6811) app-shell.tsx's own g-prefix handler owns SPA navigation on /app/* routes, but the site-wide handler in keyboard-shortcuts.tsx fired unconditionally on the same keystrokes and hard-navigated away from the app via window.location.assign, colliding on g r and g a. The site-wide handler now defers to AppShell while an /app/* route is mounted, and the advertised shortcut list swaps to the in-app destinations so it stays accurate.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-17 15:43:59 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
app-shell.tsx's document-level "g " handler (o/w/r/p/a -> SPAnavigate()) andkeyboard-shortcuts.tsx's window-level "g " handler (h/d/a/r ->window.location.assign()) are both always mounted on every/app/*page. Pressinggthenrorgthenafired both: the SPA navigation fromapp-shell.tsxand a hard, full-page navigation fromkeyboard-shortcuts.tsx, aborting the SPA transition and taking the user out of the app entirely.app-shell.tsxwhile an/app/*route is mounted (checked viauseLocation().pathname), so only one handler ever resolves a given "g " sequence.KeyboardShortcutsDialog's advertised shortcut list now also swaps to the in-app destinations (g o/g w/g r/g p/g a) while on/app/*, so the cheat sheet stays accurate instead of advertising bindings that no longer apply.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #6811
Validation
git diff --checknpm run actionlint(backend-only check; this PR only touchesapps/loopover-ui)npm run typecheck(vianpm run ui:typecheck, the UI-scoped equivalent)npm run test:coverage(backend-only; this PR only touchesapps/loopover-ui, which is outside the Codecov patch gate percodecov.yml'signore: apps/**)npm run test:workers(backend-only; not applicable to this UI-only change)npm run build:mcp(not applicable to this UI-only change)npm run test:mcp-pack(not applicable to this UI-only change)npm run ui:openapi:check(no API/schema changes in this PR)npm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderate(no dependency changes in this PR)If any required check was skipped, explain why:
apps/loopover-ui/src/components/site/keyboard-shortcuts.tsx(plus its test file). The backend-only checks above (actionlint,test:coverage,test:workers,build:mcp,test:mcp-pack) and dependency/API checks (npm audit,ui:openapi:check) have no surface to exercise for a change scoped entirely to one UI event handler, so they were not run standalone;npm run ui:lint,npm run ui:typecheck, andnpm run ui:buildall pass locally, and the newkeyboard-shortcuts.test.tsxsuite (5 tests) covers both collision keystrokes plus the advertised-shortcut-list swap.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots. (N/A -- this fix changes keyboard event-handling logic and the shortcut cheat-sheet's text list only; there is no static visual difference to screenshot, and the interaction difference -- one navigation instead of two racing navigations -- is exercised by the new automated tests instead.)Notes
keyboard-shortcuts.test.tsx, assertingwindow.location.assignis no longer called forg r/g awhile an/app/*route is mounted, and that the cheat sheet's advertised list matches whichever handler is actually live.