feat: query history + keyboard shortcuts hook - #611
Conversation
Query History (#608): - Store last 10 queries per widget in widget.settings.queryHistory[] - Dedup consecutive identical queries, skip empty, cap at 10 - Clock icon + "History" popover button in query editor panel - Click to restore previous query to the editor - History saved on widget save, persists in layoutJson - 9 unit tests for store logic Keyboard Shortcuts Hook (#609 — infrastructure only): - useKeyboardShortcuts() hook with parseShortcut/matchesShortcut - Cross-platform Cmd support (Meta on Mac, Ctrl on Win/Linux) - Suppresses shortcuts when text input/editor is focused - 10 unit tests for parsing and matching Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughImplements keyboard shortcuts for dashboard navigation (Cmd+S to save, Cmd+E to edit, Cmd+N to add widget, Escape to close) and adds a query history feature to the widget editor. Query history stores up to 10 recent queries per widget in settings with timestamps, accessible via a history dropdown in the query editor panel. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Edit mode: Cmd+S (save), Cmd+E (exit to view), Cmd+N (add widget), Escape (close modal). View mode: Cmd+E (enter edit). Button tooltips show shortcut hints. All shortcuts suppressed when typing in inputs or the query editor. Closes #609 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/components/widget-editor-modal.tsx (1)
687-757:⚠️ Potential issue | 🔴 CriticalStale
queryHistorysnapshot — the just-appended entry is dropped from the save payload.
queryHistoryat line 136 is a render-time selector snapshot. AfteraddToQueryHistory(query)(line 691) updates the store, the localqueryHistoryvariable used at lines 755–756 is still the previous render's value, so the entry just appended is not included insettings.queryHistory. Persisted history lags by one save, and on nextloadFromWidgetthe most recent query is missing.
handleRunAndSavealready does the right thing (useWidgetEditorStore.getState().queryHistoryat line 562) — mirror that here.🐛 Proposed fix
function handleSave() { const id = widget?.id ?? crypto.randomUUID(); // Record query in history before saving if (query.trim() && !isParamSelect && !isContentOnly) { addToQueryHistory(query); } + // Read fresh history after the append (component-scoped selector is stale here). + const updatedHistory = useWidgetEditorStore.getState().queryHistory; const clickAction = buildClickAction(); ... queryHistory: isParamSelect || isContentOnly ? undefined - : queryHistory.length - ? queryHistory + : updatedHistory.length + ? updatedHistory : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/widget-editor-modal.tsx` around lines 687 - 757, handleSave currently uses the render-time queryHistory variable, so after calling addToQueryHistory(query) the newly appended entry isn't included in settings.queryHistory; change the save to read the latest store snapshot (useWidgetEditorStore.getState().queryHistory) after addToQueryHistory and pass that fresh array into the onSave payload (settings.queryHistory) instead of the stale local queryHistory variable, keeping the same conditional guards used now for isParamSelect/isContentOnly.
🧹 Nitpick comments (5)
app/src/components/widget-editor/query-editor-panel.tsx (1)
34-44: Optional: lean onIntl.RelativeTimeFormatfor i18n‑friendly relative time.
formatTimeAgoworks, but hard‑codes English suffixes. If you ever localize the editor, swapping inIntl.RelativeTimeFormatwould give you free pluralization and locale support.♻️ Suggested refactor
-function formatTimeAgo(iso: string): string { - const ms = Date.now() - new Date(iso).getTime(); - const sec = Math.floor(ms / 1000); - if (sec < 60) return "just now"; - const min = Math.floor(sec / 60); - if (min < 60) return min + "m ago"; - const hr = Math.floor(min / 60); - if (hr < 24) return hr + "h ago"; - const days = Math.floor(hr / 24); - return days + "d ago"; -} +const RTF = new Intl.RelativeTimeFormat(undefined, { numeric: "auto" }); +function formatTimeAgo(iso: string): string { + const sec = Math.floor((Date.now() - new Date(iso).getTime()) / 1000); + if (sec < 60) return RTF.format(-sec, "second"); + const min = Math.floor(sec / 60); + if (min < 60) return RTF.format(-min, "minute"); + const hr = Math.floor(min / 60); + if (hr < 24) return RTF.format(-hr, "hour"); + return RTF.format(-Math.floor(hr / 24), "day"); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/widget-editor/query-editor-panel.tsx` around lines 34 - 44, The current formatTimeAgo function hardcodes English suffixes; replace its manual logic with Intl.RelativeTimeFormat to get locale-aware pluralization and units. In formatTimeAgo(iso: string) compute the elapsed seconds/minutes/hours/days as before, choose the largest appropriate unit ("second","minute","hour","day") and call new Intl.RelativeTimeFormat(undefined, {numeric: "auto"}).format(-value, unit) to return the localized relative string; keep the function name and signature unchanged so callers of formatTimeAgo continue to work.app/src/hooks/use-keyboard-shortcuts.ts (3)
95-120: Listener is re-registered on every render.Consumers pass inline arrays (
useKeyboardShortcuts([{ ... }])), soshortcutsis a new reference each render,handleKeyDowngets a new identity, and theuseEffecttears down + re-adds thekeydownlistener every render. Functional, but wasteful on dashboards that re-render frequently (parameter store, countdowns, tanstack-query ticks).A common fix is to stash the latest shortcuts in a ref and register the listener once:
♻️ Proposed fix
-import { useEffect, useCallback } from "react"; +import { useEffect, useRef } from "react"; @@ export function useKeyboardShortcuts(shortcuts: ShortcutDefinition[]): void { - const handleKeyDown = useCallback( - (event: KeyboardEvent) => { - for (const def of shortcuts) { + const shortcutsRef = useRef(shortcuts); + shortcutsRef.current = shortcuts; + + useEffect(() => { + const handleKeyDown = (event: KeyboardEvent) => { + for (const def of shortcutsRef.current) { if (def.disabled) continue; const parsed = parseShortcut(def.shortcut); if (!matchesShortcut(event, parsed)) continue; - - // Allow Escape even in inputs (to close modals) - // Suppress all other shortcuts when typing if (parsed.key !== "escape" && isInputFocused()) continue; - event.preventDefault(); event.stopPropagation(); def.handler(); return; } - }, - [shortcuts], - ); - - useEffect(() => { - document.addEventListener("keydown", handleKeyDown); - return () => document.removeEventListener("keydown", handleKeyDown); - }, [handleKeyDown]); + }; + document.addEventListener("keydown", handleKeyDown); + return () => document.removeEventListener("keydown", handleKeyDown); + }, []); }As a bonus, you could memo the parsed shortcuts so
parseShortcutdoesn't run on every keystroke.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/hooks/use-keyboard-shortcuts.ts` around lines 95 - 120, The keydown listener is being re-registered each render because handleKeyDown depends on the incoming shortcuts array; fix useKeyboardShortcuts by storing the latest shortcuts (and optionally their parsed forms) in a ref (e.g., shortcutsRef and parsedShortcutsRef) and update those refs when the shortcuts prop changes, then register a stable handleKeyDown once in a mount-only useEffect which reads from the refs; also memoize or pre-parse shortcuts when updating the ref so parseShortcut isn't called on every keystroke.
75-87: Consider also suppressing onrole="textbox"/role="searchbox".
INPUT_TAGS+isContentEditable+role="combobox"covers the current UI, but custom Radix/ARIA widgets used elsewhere (e.g., command palettes) often exposerole="textbox"orrole="searchbox". Cheap to add now before shortcuts are wired into more pages.♻️ Proposed fix
- if (el.getAttribute("role") === "combobox") return true; + const role = el.getAttribute("role"); + if (role === "combobox" || role === "textbox" || role === "searchbox") return true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/hooks/use-keyboard-shortcuts.ts` around lines 75 - 87, The isInputFocused() shortcut-suppression logic misses elements exposing role="textbox" or role="searchbox"; update the function to also treat those ARIA roles as input-focused. Modify isInputFocused() (and/or add a small INPUT_ROLES set alongside INPUT_TAGS) to check el.getAttribute("role") and return true when it equals "combobox", "textbox", or "searchbox" so keyboard shortcuts are suppressed for those widgets as well.
39-60:matchesShortcutlogic is correct but hard to follow.The branching between
shortcut.metaand the explicitshortcut.ctrlpath, plus the redundant-looking Line 55 check, took a second read to verify. Given this is the matcher powering every shortcut in the app, a short comment mapping each rule to an example (Cmd+S,Ctrl+S,Shift+Escape) would help future maintainers — and the extra test cases suggested on the test file would lock in the contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/hooks/use-keyboard-shortcuts.ts` around lines 39 - 60, The matchesShortcut function is correct but hard to follow; add concise inline comments inside matchesShortcut describing each conditional and mapping it to examples (e.g., "shortcut.meta: accepts Cmd on mac or Ctrl on Win — example: Cmd+S/Ctrl+S", "when !shortcut.meta: neither meta nor ctrl should be pressed unless shortcut.ctrl is true", "explicit ctrl check covers Ctrl on both platforms", and "shift/alt exact-match checks — e.g., Shift+Escape"), and clarify the intent of the final ctrl check (the condition using shortcut.ctrl && !event.ctrlKey && !event.metaKey) so readers know it accepts Ctrl or Cmd as the modifier. Also add unit tests for matchesShortcut covering Cmd+S, Ctrl+S, plain S, Shift+Escape, and negative cases to lock the contract; reference the matchesShortcut function and ParsedShortcut type when adding tests.app/src/hooks/__tests__/use-keyboard-shortcuts.test.ts (1)
50-98: Consider covering the Ctrl-only branch and thedisabledpath.Tests exercise
Cmd+*andEscapenicely, but the asymmetric logic inmatchesShortcutfor explicitCtrl+X(Lines 51-55 ofuse-keyboard-shortcuts.ts) and theshortcut.ctrl && metaKeybranch aren't covered. A couple of additional cases would pin the contract:🧪 Suggested extra cases
it("matches explicit Ctrl+S and rejects Cmd+S for it", () => { const parsed = parseShortcut("Ctrl+S"); expect(matchesShortcut(makeEvent({ key: "s", ctrlKey: true }), parsed)).toBe(true); expect(matchesShortcut(makeEvent({ key: "s", metaKey: true }), parsed)).toBe(false); }); it("rejects Cmd+S when Shift is also pressed", () => { const parsed = parseShortcut("Cmd+S"); expect( matchesShortcut(makeEvent({ key: "s", metaKey: true, shiftKey: true }), parsed), ).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/hooks/__tests__/use-keyboard-shortcuts.test.ts` around lines 50 - 98, Add tests that cover the explicit Ctrl-only branch and the disabled path in matchesShortcut: create a parsed shortcut via parseShortcut("Ctrl+S") and assert matchesShortcut(makeEvent({ key: "s", ctrlKey: true }), parsed) is true while matchesShortcut(makeEvent({ key: "s", metaKey: true }), parsed) is false; also add a test that asserts parseShortcut("Cmd+S") does not match when an extra modifier is present (e.g., shiftKey true) and a test that passes a disabled shortcut/event into matchesShortcut (or calls the handler with disabled true) to assert it returns false; reference matchesShortcut, parseShortcut and the test helper makeEvent to locate where to add these cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/app/`(dashboard)/[id]/edit/page.tsx:
- Around line 341-346: The Cmd+E shortcut handler currently calls
router.push("/" + id) which bypasses the unsaved-changes protection; change the
handler to invoke the same navigation helper used by the Back button
(requestNavigation(`/${id}`) or the local function that wraps unsaved-change
checks) instead of calling router.push directly so the unsaved-changes prompt
appears before navigating away.
- Around line 352-357: The Escape shortcut currently always prevents the event
because use-keyboard-shortcuts calls preventDefault when matchesShortcut is
true; change the shortcut entry in page.tsx to include a disabled flag that
mirrors the editorOpen state (e.g., disabled: !editorOpen) so the key is only
consumed when the editor is open; update the object with the unique shortcut
config (shortcut: "Escape", handler: ... ) to add disabled and you can simplify
the handler to just setEditorOpen(false) without the editorOpen check.
- Around line 347-351: Remove the browser-reserved "Cmd+N" binding in the
keyboard shortcut object (the entry using shortcut: "Cmd+N", handler:
openAddWidget, disabled: editorOpen) and replace it with an unreserved combo
such as "Cmd+Option+N" or "Cmd+;" to ensure the shortcut can be intercepted;
keep the handler (openAddWidget) and disabled flag (editorOpen) unchanged. Also
update the add-widget button tooltip text referenced elsewhere in this file (the
tooltip that currently mentions Cmd+N) to reflect the new shortcut string so the
UI and key binding remain consistent.
---
Outside diff comments:
In `@app/src/components/widget-editor-modal.tsx`:
- Around line 687-757: handleSave currently uses the render-time queryHistory
variable, so after calling addToQueryHistory(query) the newly appended entry
isn't included in settings.queryHistory; change the save to read the latest
store snapshot (useWidgetEditorStore.getState().queryHistory) after
addToQueryHistory and pass that fresh array into the onSave payload
(settings.queryHistory) instead of the stale local queryHistory variable,
keeping the same conditional guards used now for isParamSelect/isContentOnly.
---
Nitpick comments:
In `@app/src/components/widget-editor/query-editor-panel.tsx`:
- Around line 34-44: The current formatTimeAgo function hardcodes English
suffixes; replace its manual logic with Intl.RelativeTimeFormat to get
locale-aware pluralization and units. In formatTimeAgo(iso: string) compute the
elapsed seconds/minutes/hours/days as before, choose the largest appropriate
unit ("second","minute","hour","day") and call new
Intl.RelativeTimeFormat(undefined, {numeric: "auto"}).format(-value, unit) to
return the localized relative string; keep the function name and signature
unchanged so callers of formatTimeAgo continue to work.
In `@app/src/hooks/__tests__/use-keyboard-shortcuts.test.ts`:
- Around line 50-98: Add tests that cover the explicit Ctrl-only branch and the
disabled path in matchesShortcut: create a parsed shortcut via
parseShortcut("Ctrl+S") and assert matchesShortcut(makeEvent({ key: "s",
ctrlKey: true }), parsed) is true while matchesShortcut(makeEvent({ key: "s",
metaKey: true }), parsed) is false; also add a test that asserts
parseShortcut("Cmd+S") does not match when an extra modifier is present (e.g.,
shiftKey true) and a test that passes a disabled shortcut/event into
matchesShortcut (or calls the handler with disabled true) to assert it returns
false; reference matchesShortcut, parseShortcut and the test helper makeEvent to
locate where to add these cases.
In `@app/src/hooks/use-keyboard-shortcuts.ts`:
- Around line 95-120: The keydown listener is being re-registered each render
because handleKeyDown depends on the incoming shortcuts array; fix
useKeyboardShortcuts by storing the latest shortcuts (and optionally their
parsed forms) in a ref (e.g., shortcutsRef and parsedShortcutsRef) and update
those refs when the shortcuts prop changes, then register a stable handleKeyDown
once in a mount-only useEffect which reads from the refs; also memoize or
pre-parse shortcuts when updating the ref so parseShortcut isn't called on every
keystroke.
- Around line 75-87: The isInputFocused() shortcut-suppression logic misses
elements exposing role="textbox" or role="searchbox"; update the function to
also treat those ARIA roles as input-focused. Modify isInputFocused() (and/or
add a small INPUT_ROLES set alongside INPUT_TAGS) to check
el.getAttribute("role") and return true when it equals "combobox", "textbox", or
"searchbox" so keyboard shortcuts are suppressed for those widgets as well.
- Around line 39-60: The matchesShortcut function is correct but hard to follow;
add concise inline comments inside matchesShortcut describing each conditional
and mapping it to examples (e.g., "shortcut.meta: accepts Cmd on mac or Ctrl on
Win — example: Cmd+S/Ctrl+S", "when !shortcut.meta: neither meta nor ctrl should
be pressed unless shortcut.ctrl is true", "explicit ctrl check covers Ctrl on
both platforms", and "shift/alt exact-match checks — e.g., Shift+Escape"), and
clarify the intent of the final ctrl check (the condition using shortcut.ctrl &&
!event.ctrlKey && !event.metaKey) so readers know it accepts Ctrl or Cmd as the
modifier. Also add unit tests for matchesShortcut covering Cmd+S, Ctrl+S, plain
S, Shift+Escape, and negative cases to lock the contract; reference the
matchesShortcut function and ParsedShortcut type when adding tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15d4532e-66dc-41b4-8e76-584a9072c078
📒 Files selected for processing (8)
app/src/app/(dashboard)/[id]/edit/page.tsxapp/src/app/(dashboard)/[id]/page.tsxapp/src/components/widget-editor-modal.tsxapp/src/components/widget-editor/query-editor-panel.tsxapp/src/hooks/__tests__/use-keyboard-shortcuts.test.tsapp/src/hooks/use-keyboard-shortcuts.tsapp/src/stores/__tests__/widget-editor-store.test.tsapp/src/stores/widget-editor-store.ts
| { | ||
| shortcut: "Cmd+E", | ||
| handler: () => { | ||
| router.push("/" + id); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Cmd+E bypasses the unsaved-changes warning.
The "Back" button at Line 403 routes through requestNavigation(\/${id}`)so users are prompted when there are unsaved changes. This shortcut callsrouter.push` directly, so a stray Cmd+E silently discards work.
🛡️ Proposed fix
{
shortcut: "Cmd+E",
handler: () => {
- router.push("/" + id);
+ if (requestNavigation(`/${id}`)) router.push(`/${id}`);
},
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| shortcut: "Cmd+E", | |
| handler: () => { | |
| router.push("/" + id); | |
| }, | |
| }, | |
| { | |
| shortcut: "Cmd+E", | |
| handler: () => { | |
| if (requestNavigation(`/${id}`)) router.push(`/${id}`); | |
| }, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/app/`(dashboard)/[id]/edit/page.tsx around lines 341 - 346, The Cmd+E
shortcut handler currently calls router.push("/" + id) which bypasses the
unsaved-changes protection; change the handler to invoke the same navigation
helper used by the Back button (requestNavigation(`/${id}`) or the local
function that wraps unsaved-change checks) instead of calling router.push
directly so the unsaved-changes prompt appears before navigating away.
| { | ||
| shortcut: "Cmd+N", | ||
| handler: openAddWidget, | ||
| disabled: editorOpen, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Can a web page intercept Cmd+N (Mac) or Ctrl+N keydown via event.preventDefault in Chrome, Safari, Firefox?
💡 Result:
No, a web page cannot intercept Cmd+N (Mac) or Ctrl+N using event.preventDefault in Chrome, Safari, or Firefox. These are reserved browser shortcuts for opening a new window, and browsers intentionally prevent web content from overriding them to protect user control. Chrome: Reserves Ctrl+N (and Cmd+N on Mac) since Chrome 4; preventDefault has no effect. Sources confirm this policy persists. Safari: Cmd+N opens a new window (or tab depending on settings); multiple sources state CMD events like this cannot be overridden with preventDefault. Firefox: Ctrl+N/Cmd+N is unpreventable; even the keydown event may not fire for the 'N' key in some cases, and Bugzilla confirms it's by design (WONTFIX). Workarounds like Chrome apps or kiosk mode exist but are not standard web pages.
Citations:
- 1: https://stackoverflow.com/questions/42596142/unpreventable-keyboard-shortcuts-in-javascript
- 2: https://stackoverflow.com/questions/32390157/shortcut-key-commands-in-chrome-not-preventing-default-action
- 3: https://stackoverflow.com/questions/38838302/any-way-to-override-ctrln-to-open-a-new-window-in-chrome
- 4: https://stackoverflow.com/questions/7295508/javascript-capture-browser-shortcuts-ctrlt-n-w
- 5: https://stackoverflow.com/questions/44998250/which-chrome-keyboard-shortcuts-cannot-be-overridden-with-javascript
- 6: https://bugzilla.mozilla.org/show_bug.cgi?id=473360
🏁 Script executed:
# Verify the file exists and check the code at the specified lines
cat -n app/src/app/\(dashboard\)/\[id\]/edit/page.tsx | sed -n '340,360p'Repository: alfredo1996/neoboard
Length of output: 601
🏁 Script executed:
# Check line 471 for the tooltip mentioned
cat -n app/src/app/\(dashboard\)/\[id\]/edit/page.tsx | sed -n '465,477p'Repository: alfredo1996/neoboard
Length of output: 560
🏁 Script executed:
# Search for other keyboard shortcut definitions to check for conflicts
rg -n "shortcut:" app/src/app/\(dashboard\)/\[id\]/edit/page.tsxRepository: alfredo1996/neoboard
Length of output: 181
🏁 Script executed:
# Search for all keyboard shortcut definitions across the app
rg -n "shortcut.*:" app/src --type tsx --type ts -A 2 | head -60Repository: alfredo1996/neoboard
Length of output: 92
🌐 Web query:
Are Cmd+Shift+N and Cmd+K reserved shortcuts in Chrome, Safari, Firefox browsers?
💡 Result:
Yes, Cmd+Shift+N and Cmd+K are reserved shortcuts in Chrome, Safari, and Firefox browsers on macOS. Cmd+Shift+N: - Chrome: Opens a new Incognito window (official docs, results 2,3,28). - Firefox: Reopens the last closed window (results 1,20,22). - Safari: Opens a new Private window (system Finder shortcut repurposed, results 11; new window is Cmd+N or Option+Cmd+N). Cmd+K: - Chrome: Focuses the address bar for search (results 9,10,22,28). - Firefox: Focuses search bar or address bar for web search (results 20,21,22). - Safari: Not a primary browser shortcut; used in Finder for Connect to Server and some app-specific functions, but not conflicting in browser context (results 11,14,24,26).
Citations:
- 1: https://www.reddit.com/r/firefox/comments/1m6oxke/warning_cmdshiftn_opens_the_last_closed_window/
- 2: https://support.google.com/chrome/answer/157179?co=GENIE.Platform%3DDesktop&hl=en
- 3: https://support.google.com/chrome/answer/157179
- 4: https://www.reddit.com/r/programming/comments/11de9il/why_is_cmdctrlk_the_hotkey_to_search_on_many/
- 5: Google Chrome & Windows -
CTRL+Kforces a google search dip/cmdk#288 - 6: https://support.apple.com/kb/HT201236
- 7: https://support.apple.com/guide/safari/keyboard-shortcuts-and-gestures-cpsh003/mac
- 8: https://www.idownloadblog.com/2020/04/13/keyboard-shortcuts-for-firefox-mac/
- 9: https://www-ist.massey.ac.nz/kamercer/FirefoxShortcuts.htm
- 10: https://support.mozilla.org/en-US/kb/Keyboard%20shortcuts
- 11: https://support.apple.com/en-am/guide/safari/cpsh003/mac
- 12: https://www.dca.ca.gov/about_us/kbs_safari.shtml
- 13: https://support.google.com/chrome/answer/157179?hl=en&co=GENIE.Platform%3DDesktop&oco=0
Remove the Cmd+N keyboard shortcut; it's browser-reserved and uninterceptable.
Cmd+N (and Ctrl+N on Windows/Linux) are reserved by Chrome, Safari, and Firefox for opening a new window and cannot be intercepted via preventDefault(). The suggested alternatives in development are also problematic: Cmd+Shift+N triggers Incognito mode in Chrome and window-reopening in Firefox; Cmd+K focuses the address bar in Chrome and Firefox. Consider using an unreserved binding (e.g., Cmd+Option+N or Cmd+;). Update the tooltip at line 471 accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/app/`(dashboard)/[id]/edit/page.tsx around lines 347 - 351, Remove
the browser-reserved "Cmd+N" binding in the keyboard shortcut object (the entry
using shortcut: "Cmd+N", handler: openAddWidget, disabled: editorOpen) and
replace it with an unreserved combo such as "Cmd+Option+N" or "Cmd+;" to ensure
the shortcut can be intercepted; keep the handler (openAddWidget) and disabled
flag (editorOpen) unchanged. Also update the add-widget button tooltip text
referenced elsewhere in this file (the tooltip that currently mentions Cmd+N) to
reflect the new shortcut string so the UI and key binding remain consistent.
| { | ||
| shortcut: "Escape", | ||
| handler: () => { | ||
| if (editorOpen) setEditorOpen(false); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Escape always preventDefaults, even when the editor is closed.
The hook calls event.preventDefault() / stopPropagation() as soon as matchesShortcut returns true (see app/src/hooks/use-keyboard-shortcuts.ts Lines 107-109), before the handler body runs. When editorOpen is false the handler is a no-op but the event is still swallowed, which can shadow Escape handlers for the Sharing Sheet, ConfirmDialog, page tabs, etc.
Prefer gating with disabled so the event is only consumed when actually handled:
♻️ Proposed fix
{
shortcut: "Escape",
- handler: () => {
- if (editorOpen) setEditorOpen(false);
- },
+ handler: () => setEditorOpen(false),
+ disabled: !editorOpen,
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/app/`(dashboard)/[id]/edit/page.tsx around lines 352 - 357, The
Escape shortcut currently always prevents the event because
use-keyboard-shortcuts calls preventDefault when matchesShortcut is true; change
the shortcut entry in page.tsx to include a disabled flag that mirrors the
editorOpen state (e.g., disabled: !editorOpen) so the key is only consumed when
the editor is open; update the object with the unique shortcut config (shortcut:
"Escape", handler: ... ) to add disabled and you can simplify the handler to
just setEditorOpen(false) without the editorOpen check.
|
1. Transform error warning banner 2. Cmd+E unsaved-changes guard 3. Cmd+N → Cmd+Shift+N (browser-reserved) 4. Escape disabled when editor not open 5. Choropleth import unmount safety 6. Choropleth transform collision guard Co-authored-by: alfredorubin96 <alfredo.rubin@neotechnology.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
Two features in one PR:
Query History (#608)
widget.settings.queryHistory[]Keyboard Shortcuts Hook (#609 — infrastructure)
useKeyboardShortcuts()hook withparseShortcut/matchesShortcutCmdmaps to Meta (Mac) or Ctrl (Win/Linux)Test plan
Closes #608
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests