fix(command-line): hide Ctrl/Cmd + Shift + P hint on Firefox (@d1rshan)#7690
fix(command-line): hide Ctrl/Cmd + Shift + P hint on Firefox (@d1rshan)#7690d1rshan wants to merge 1 commit intomonkeytypegame:masterfrom
Conversation
github-actions
bot
added
the
waiting for review
There was a problem hiding this comment.
Pull request overview
Adjusts frontend UI shortcut hints to avoid advertising Ctrl/Cmd + Shift + P on Firefox, where that shortcut is reserved by the browser and can’t be used reliably for opening Monkeytype’s command line.
Changes:
- Hide
Ctrl/Cmd + Shift + Pin shortcut hint text when running on Firefox. - Continue showing the
Esc/Tab(depending on quick restart setting) hint in Settings, Footer keytips, and About page.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/ts/pages/settings.ts | Updates Settings tip HTML to omit the Ctrl/Cmd + Shift + P hint on Firefox. |
| frontend/src/ts/components/pages/AboutPage.tsx | Updates About page “keybinds” copy to conditionally omit the Ctrl/Cmd + Shift + P hint on Firefox. |
| frontend/src/ts/components/layout/footer/Keytips.tsx | Updates footer keytips to conditionally omit the Ctrl/Cmd + Shift + P hint on Firefox. |
| const userAgent = window.navigator.userAgent.toLowerCase(); | ||
| const modifierKey = | ||
| userAgent.includes("mac") && !userAgent.includes("firefox") | ||
| ? "cmd" | ||
| : "ctrl"; | ||
| const isFirefox = userAgent.includes("firefox"); | ||
| const modifierKey = userAgent.includes("mac") && !isFirefox ? "cmd" : "ctrl"; | ||
|
|
||
| const commandKey = (): string => | ||
| getConfig.quickRestart === "esc" ? "tab" : "esc"; |
There was a problem hiding this comment.
isFirefox/modifierKey user-agent parsing is duplicated across this PR (also in settings.ts/AboutPage.tsx). Consider extracting a small shared helper (e.g., utils/browser or utils/shortcuts) so future browser/platform tweaks only need changing in one place.
| qs(".pageSettings .tip")?.setHtml(` | ||
| tip: You can also change all these settings quickly using the | ||
| command line (<kbd>${commandKey}</kbd> or <kbd>${modifierKey}</kbd> + <kbd>shift</kbd> + <kbd>p</kbd>)`); | ||
| command line (${isFirefox ? `<kbd>${commandKey}</kbd>` : `<kbd>${commandKey}</kbd> or <kbd>${modifierKey}</kbd> + <kbd>shift</kbd> + <kbd>p</kbd>`})`); |
There was a problem hiding this comment.
The setHtml template literal now contains a nested ternary with nested template literals, which is hard to scan/maintain. Consider building the shortcut hint string in a variable first, then interpolating it into the final setHtml call.
| export function AboutPage(): JSXElement { | ||
| const isOpen = () => getActivePage() === "about"; | ||
| const isFirefox = window.navigator.userAgent | ||
| .toLowerCase() | ||
| .includes("firefox"); |
There was a problem hiding this comment.
Browser detection via window.navigator.userAgent.toLowerCase().includes("firefox") is now repeated in multiple UI files in this PR. Consider centralizing it in a shared helper to avoid drift/inconsistent behavior over time.
| export function AboutPage(): JSXElement { | |
| const isOpen = () => getActivePage() === "about"; | |
| const isFirefox = window.navigator.userAgent | |
| .toLowerCase() | |
| .includes("firefox"); | |
| const isFirefoxBrowser = (): boolean => | |
| typeof window !== "undefined" && | |
| window.navigator.userAgent.toLowerCase().includes("firefox"); | |
| export function AboutPage(): JSXElement { | |
| const isOpen = () => getActivePage() === "about"; | |
| const isFirefox = isFirefoxBrowser(); |
Summary
Hide the Cmd/Ctrl + Shift + P shortcut on Firefox, where it is reserved for private windows and not usable by the app.
Changes
Notes
UI-only change. No changes to keybind logic.
Testing
Closes #7689