fix(web): close settings with Escape#916
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
the
vouch:unvouched
| useEffect(() => { | ||
| const onWindowKeyDown = (event: KeyboardEvent) => { | ||
| if (!shouldCloseSettingsOnEscape(event)) { | ||
| return; | ||
| } | ||
|
|
||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
|
|
||
| if (canNavigateBackInApp(window.history.state)) { | ||
| window.history.back(); | ||
| return; | ||
| } | ||
|
|
||
| void navigate({ to: "/", replace: true }); | ||
| }; | ||
|
|
||
| window.addEventListener("keydown", onWindowKeyDown); | ||
| return () => { | ||
| window.removeEventListener("keydown", onWindowKeyDown); | ||
| }; | ||
| }, [navigate]); |
There was a problem hiding this comment.
this will conflict with the global keybinding system pretty sure
There was a problem hiding this comment.
no longer the case with new sidebar layout.
fixed in #1503
2 participants
What Changed
/for direct-entry casesWhy
CmdOrCtrl+,already opens settings in desktop builds, but there was no matching keyboard path to leave that viewUI Changes
Validation
bun fmtbun lintbun typecheckbun run test src/lib/settingsNavigation.test.tsChecklist
Note
Close settings view on Escape key press
Adds a
keydownlistener in_chat.settings.tsxthat closes the settings view when an unmodified, unprevented Escape key is pressed. If in-app back navigation is available (detected via TanStack Router's__TSR_indexinwindow.history.state), it callswindow.history.back(); otherwise it navigates to/withreplace: true. Two utility functions,shouldCloseSettingsOnEscapeandcanNavigateBackInApp, are extracted tosettingsNavigation.tswith corresponding Vitest tests.Macroscope summarized 1c79239.
Note
Low Risk
Adds a global Escape key handler to the settings route that manipulates browser history/navigation; limited scope but could affect keyboard shortcut propagation and routing behavior.
Overview
Enables dismissing the settings view via an unmodified
Escapekeypress._chat.settings.tsxnow installs a windowkeydownlistener that prevents/stops the event, navigatesback()when TanStack Router history indicates an in-app previous entry, and otherwise replaces the route with/.Adds
settingsNavigation.tswith small helpers (shouldCloseSettingsOnEscape,canNavigateBackInApp) plus Vitest coverage for Escape matching and history-state detection.Written by Cursor Bugbot for commit 1c79239. This will update automatically on new commits. Configure here.