feat: add Cmd+G shortcut to navigate to a note#186
Open
dalelane wants to merge 1 commit into
Open
Conversation
Adds a global Cmd+G (Ctrl+G on Windows/Linux) shortcut that opens a searchable popup to jump to any note without modifying the current page. When a note is open the popup appears inline near the cursor; when no note is open it shows as a centered overlay. This is based on the link editor (Cmd+K) that is used to insert a link to another wiki page - copying the same basic control and behaviour but making it available as a keyboard-shortcut friendly navigation interaction. I've also updated the Sidebar so that the highlighting for the currently-selected page reflects the right page after a Cmd+G. This was arguably a separate issue as this feels like it was a bug affecting clicking on Wikilinks on pages, too. Signed-off-by: Dale Lane <dale.lane@gmail.com>
📝 WalkthroughWalkthroughAdds a "note navigator" feature: a new searchable, keyboard-navigable NoteNavigator component, a Cmd/Ctrl+G shortcut opening an App-level overlay or dispatching a navigate-to-note event, Editor.tsx inline popup integration anchored at the cursor, shortcut list registration, and a Sidebar selection-sync effect. ChangesNote Navigator Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AppContent
participant Editor
participant NoteNavigator
User->>AppContent: press Cmd/Ctrl+G
alt no current note
AppContent->>NoteNavigator: open overlay
User->>NoteNavigator: type query / select note
NoteNavigator->>AppContent: onSelect(note)
AppContent->>AppContent: selectNote(note.id)
else current note open in editor
AppContent->>Editor: dispatch navigate-to-note event
Editor->>NoteNavigator: open popup at cursor
User->>NoteNavigator: select note
NoteNavigator->>Editor: onSelect(note)
Editor->>Editor: selectNote(note.id)
Editor->>NoteNavigator: destroy popup
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.tsx (1)
199-300: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBlock global shortcuts while overlays are open
noteNavigatorOpenis missing from the windowkeydownhandler, so Cmd+N/Cmd+\ and similar shortcuts still fire while the note navigator is open. The command palette and AI modal have the same gap; add a shared guard before the shortcut checks, or suppress these keys at the overlay level, so background actions don't run under any modal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.tsx` around lines 199 - 300, The global keydown handling in App.tsx is missing a shared overlay guard, so shortcuts still fire while noteNavigatorOpen, the command palette, or the AI modal are visible. Update handleKeyDown to short-circuit background shortcuts whenever any overlay/modal is open, or move the suppression into the overlay components themselves, and make sure the shortcut checks around toggleSettings, setPaletteOpen, and navigate-to-note are protected consistently.
🧹 Nitpick comments (2)
src/components/editor/NoteNavigator.tsx (2)
92-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd ARIA semantics for keyboard-driven selection.
The list items use
role="button"with no indication of the currently highlighted item for assistive tech; arrow-key navigation only conveys selection visually via background color. Usingrole="listbox"/role="option"witharia-selected/aria-activedescendanton the input would make the keyboard-driven selection perceivable by screen readers.♿ Proposed ARIA improvements
<div ref={listRef} className="p-1.5 max-h-60 overflow-y-auto flex flex-col gap-0.5" + role="listbox" > {filtered.length === 0 ? ( <div className="text-sm text-text-muted px-3 py-2"> No matching notes </div> ) : ( filtered.map((note, i) => ( <div key={note.id} data-index={i} - role="button" + role="option" + aria-selected={selectedIndex === i} tabIndex={-1} onClick={() => onSelect(note)}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/NoteNavigator.tsx` around lines 92 - 118, The keyboard-selectable note items in NoteNavigator currently rely on visual highlighting only, so update the accessibility semantics for the filtered list. Replace the per-item `role="button"` approach with a listbox/option pattern in the `filtered.map` render, add `aria-selected` to the active item, and wire the controlling input to use `aria-activedescendant` so the highlighted note is announced to screen readers during arrow-key navigation.
21-29: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider debouncing the search filter per project convention.
Filtering runs on every keystroke via
onChange(Line 73) with no debounce. As per coding guidelines,src/**/*.{ts,tsx}should "Debounce user-triggered operations: auto-save 300ms, search 150ms, file watcher 500ms, git status 1000ms." The current filter is a cheap in-memory substring match capped at 10 results, so real-world impact is minor, but a 150ms debounce would align with the established convention and guard against future growth of the notes list.♻️ Proposed debounce for the query filter
- const filtered = useMemo( - () => - notes - .filter((n) => - cleanTitle(n.title).toLowerCase().includes(query.toLowerCase()), - ) - .slice(0, 10), - [notes, query], - ); + const [debouncedQuery, setDebouncedQuery] = useState(query); + useEffect(() => { + const t = setTimeout(() => setDebouncedQuery(query), 150); + return () => clearTimeout(t); + }, [query]); + + const filtered = useMemo( + () => + notes + .filter((n) => + cleanTitle(n.title).toLowerCase().includes(debouncedQuery.toLowerCase()), + ) + .slice(0, 10), + [notes, debouncedQuery], + );Also applies to: 73-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/NoteNavigator.tsx` around lines 21 - 29, The note search in NoteNavigator currently filters on every keystroke without the project’s required debounce. Update the query handling in NoteNavigator (the useMemo filter driven by query and the onChange path that updates it) so search input is debounced by 150ms before filtering notes. Keep the existing cleanTitle-based substring match and result cap, but ensure the debounced value is what drives the filtered list.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/editor/Editor.tsx`:
- Around line 1696-1760: The note navigator popup in handleGoToNote is anchoring
to stale cursor coordinates because coordsAtPos is computed once before tippy is
created, so it won’t track scroll or resize changes. Move the coordsAtPos lookup
into the getReferenceClientRect callback (matching handleAddLink,
handleAddBlockMath, and handleEditBlockMath) so the rect is recomputed whenever
Tippy queries it. While touching this area, consider reusing the existing
closeNoteNavigatorPopup helper in the toggle-close path and the
onSelect/onCancel callbacks instead of duplicating destroy-and-null logic.
In `@src/components/layout/Sidebar.tsx`:
- Around line 205-207: The selection-reset branch in Sidebar.tsx clears
multiSelectedNoteIds but leaves lastClickedNoteId pointing at the old note, so
update the same else path that handles selectedNoteId becoming falsy to also
clear lastClickedNoteId. Use the Sidebar selection state logic around
setMultiSelectedNoteIds and lastClickedNoteId so the shift-click anchor is reset
whenever selection is dropped.
---
Outside diff comments:
In `@src/App.tsx`:
- Around line 199-300: The global keydown handling in App.tsx is missing a
shared overlay guard, so shortcuts still fire while noteNavigatorOpen, the
command palette, or the AI modal are visible. Update handleKeyDown to
short-circuit background shortcuts whenever any overlay/modal is open, or move
the suppression into the overlay components themselves, and make sure the
shortcut checks around toggleSettings, setPaletteOpen, and navigate-to-note are
protected consistently.
---
Nitpick comments:
In `@src/components/editor/NoteNavigator.tsx`:
- Around line 92-118: The keyboard-selectable note items in NoteNavigator
currently rely on visual highlighting only, so update the accessibility
semantics for the filtered list. Replace the per-item `role="button"` approach
with a listbox/option pattern in the `filtered.map` render, add `aria-selected`
to the active item, and wire the controlling input to use
`aria-activedescendant` so the highlighted note is announced to screen readers
during arrow-key navigation.
- Around line 21-29: The note search in NoteNavigator currently filters on every
keystroke without the project’s required debounce. Update the query handling in
NoteNavigator (the useMemo filter driven by query and the onChange path that
updates it) so search input is debounced by 150ms before filtering notes. Keep
the existing cleanTitle-based substring match and result cap, but ensure the
debounced value is what drives the filtered list.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 298cdd99-06bc-497c-b354-db2f678bb369
📒 Files selected for processing (5)
src/App.tsxsrc/components/editor/Editor.tsxsrc/components/editor/NoteNavigator.tsxsrc/components/layout/Sidebar.tsxsrc/lib/shortcuts.ts
Comment on lines
+1696
to
+1760
| // Navigate to note handler - show inline popup at cursor position | ||
| const handleGoToNote = useCallback(() => { | ||
| if (!editor) return; | ||
|
|
||
| closeBlockMathPopup(); | ||
| closeLinkPopup(); | ||
|
|
||
| // Toggle: second Cmd+G closes the popup | ||
| if (noteNavigatorPopupRef.current) { | ||
| noteNavigatorPopupRef.current.destroy(); | ||
| noteNavigatorPopupRef.current = null; | ||
| editor.commands.focus(); | ||
| return; | ||
| } | ||
|
|
||
| const currentNotes = notesRef.current ?? []; | ||
| const { from } = editor.state.selection; | ||
| const coords = editor.view.coordsAtPos(from); | ||
| const virtualElement = { | ||
| getBoundingClientRect: () => | ||
| ({ | ||
| width: 0, | ||
| height: 0, | ||
| top: coords.top, | ||
| left: coords.left, | ||
| right: coords.right, | ||
| bottom: coords.bottom, | ||
| x: coords.left, | ||
| y: coords.top, | ||
| toJSON: () => ({}), | ||
| }) as DOMRect, | ||
| }; | ||
|
|
||
| const component = new ReactRenderer(NoteNavigator, { | ||
| props: { | ||
| notes: currentNotes, | ||
| onSelect: (note: NoteMetadata) => { | ||
| noteNavigatorPopupRef.current?.destroy(); | ||
| noteNavigatorPopupRef.current = null; | ||
| notesCtxRef.current?.selectNote(note.id); | ||
| }, | ||
| onCancel: () => { | ||
| editor.commands.focus(); | ||
| noteNavigatorPopupRef.current?.destroy(); | ||
| noteNavigatorPopupRef.current = null; | ||
| }, | ||
| }, | ||
| editor, | ||
| }); | ||
|
|
||
| noteNavigatorPopupRef.current = tippy(document.body, { | ||
| getReferenceClientRect: () => | ||
| virtualElement.getBoundingClientRect() as DOMRect, | ||
| appendTo: () => document.body, | ||
| content: component.element, | ||
| showOnCreate: true, | ||
| interactive: true, | ||
| trigger: "manual", | ||
| placement: "bottom-start", | ||
| offset: [0, 8], | ||
| onDestroy: () => { | ||
| component.destroy(); | ||
| }, | ||
| }); | ||
| }, [editor, closeBlockMathPopup, closeLinkPopup]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Popup anchor uses stale coordinates; won't reposition on scroll/resize.
coords is computed once at line 1713, outside getBoundingClientRect, so the returned rect is frozen at the moment handleGoToNote runs. Every other cursor-anchored popup in this file (handleAddLink, handleAddBlockMath, handleEditBlockMath) recomputes coordsAtPos/domAtPos inside the getBoundingClientRect callback so the popup stays anchored if the editor scrolls or the window resizes while it's open.
🐛 Proposed fix
const currentNotes = notesRef.current ?? [];
- const { from } = editor.state.selection;
- const coords = editor.view.coordsAtPos(from);
const virtualElement = {
- getBoundingClientRect: () =>
- ({
+ getBoundingClientRect: () => {
+ const { from } = editor.state.selection;
+ const coords = editor.view.coordsAtPos(from);
+ return {
width: 0,
height: 0,
top: coords.top,
left: coords.left,
right: coords.right,
bottom: coords.bottom,
x: coords.left,
y: coords.top,
toJSON: () => ({}),
- }) as DOMRect,
+ } as DOMRect;
+ },
};Separately, the toggle-close branch (1704-1709) and the onSelect/onCancel callbacks (1733-1734, 1739-1740) manually re-implement closeNoteNavigatorPopup's destroy-and-null logic instead of calling it — worth consolidating for consistency, though low priority.
📝 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.
Suggested change
| // Navigate to note handler - show inline popup at cursor position | |
| const handleGoToNote = useCallback(() => { | |
| if (!editor) return; | |
| closeBlockMathPopup(); | |
| closeLinkPopup(); | |
| // Toggle: second Cmd+G closes the popup | |
| if (noteNavigatorPopupRef.current) { | |
| noteNavigatorPopupRef.current.destroy(); | |
| noteNavigatorPopupRef.current = null; | |
| editor.commands.focus(); | |
| return; | |
| } | |
| const currentNotes = notesRef.current ?? []; | |
| const { from } = editor.state.selection; | |
| const coords = editor.view.coordsAtPos(from); | |
| const virtualElement = { | |
| getBoundingClientRect: () => | |
| ({ | |
| width: 0, | |
| height: 0, | |
| top: coords.top, | |
| left: coords.left, | |
| right: coords.right, | |
| bottom: coords.bottom, | |
| x: coords.left, | |
| y: coords.top, | |
| toJSON: () => ({}), | |
| }) as DOMRect, | |
| }; | |
| const component = new ReactRenderer(NoteNavigator, { | |
| props: { | |
| notes: currentNotes, | |
| onSelect: (note: NoteMetadata) => { | |
| noteNavigatorPopupRef.current?.destroy(); | |
| noteNavigatorPopupRef.current = null; | |
| notesCtxRef.current?.selectNote(note.id); | |
| }, | |
| onCancel: () => { | |
| editor.commands.focus(); | |
| noteNavigatorPopupRef.current?.destroy(); | |
| noteNavigatorPopupRef.current = null; | |
| }, | |
| }, | |
| editor, | |
| }); | |
| noteNavigatorPopupRef.current = tippy(document.body, { | |
| getReferenceClientRect: () => | |
| virtualElement.getBoundingClientRect() as DOMRect, | |
| appendTo: () => document.body, | |
| content: component.element, | |
| showOnCreate: true, | |
| interactive: true, | |
| trigger: "manual", | |
| placement: "bottom-start", | |
| offset: [0, 8], | |
| onDestroy: () => { | |
| component.destroy(); | |
| }, | |
| }); | |
| }, [editor, closeBlockMathPopup, closeLinkPopup]); | |
| // Navigate to note handler - show inline popup at cursor position | |
| const handleGoToNote = useCallback(() => { | |
| if (!editor) return; | |
| closeBlockMathPopup(); | |
| closeLinkPopup(); | |
| // Toggle: second Cmd+G closes the popup | |
| if (noteNavigatorPopupRef.current) { | |
| noteNavigatorPopupRef.current.destroy(); | |
| noteNavigatorPopupRef.current = null; | |
| editor.commands.focus(); | |
| return; | |
| } | |
| const currentNotes = notesRef.current ?? []; | |
| const virtualElement = { | |
| getBoundingClientRect: () => { | |
| const { from } = editor.state.selection; | |
| const coords = editor.view.coordsAtPos(from); | |
| return { | |
| width: 0, | |
| height: 0, | |
| top: coords.top, | |
| left: coords.left, | |
| right: coords.right, | |
| bottom: coords.bottom, | |
| x: coords.left, | |
| y: coords.top, | |
| toJSON: () => ({}), | |
| } as DOMRect; | |
| }, | |
| }; | |
| const component = new ReactRenderer(NoteNavigator, { | |
| props: { | |
| notes: currentNotes, | |
| onSelect: (note: NoteMetadata) => { | |
| noteNavigatorPopupRef.current?.destroy(); | |
| noteNavigatorPopupRef.current = null; | |
| notesCtxRef.current?.selectNote(note.id); | |
| }, | |
| onCancel: () => { | |
| editor.commands.focus(); | |
| noteNavigatorPopupRef.current?.destroy(); | |
| noteNavigatorPopupRef.current = null; | |
| }, | |
| }, | |
| editor, | |
| }); | |
| noteNavigatorPopupRef.current = tippy(document.body, { | |
| getReferenceClientRect: () => | |
| virtualElement.getBoundingClientRect() as DOMRect, | |
| appendTo: () => document.body, | |
| content: component.element, | |
| showOnCreate: true, | |
| interactive: true, | |
| trigger: "manual", | |
| placement: "bottom-start", | |
| offset: [0, 8], | |
| onDestroy: () => { | |
| component.destroy(); | |
| }, | |
| }); | |
| }, [editor, closeBlockMathPopup, closeLinkPopup]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/editor/Editor.tsx` around lines 1696 - 1760, The note
navigator popup in handleGoToNote is anchoring to stale cursor coordinates
because coordsAtPos is computed once before tippy is created, so it won’t track
scroll or resize changes. Move the coordsAtPos lookup into the
getReferenceClientRect callback (matching handleAddLink, handleAddBlockMath, and
handleEditBlockMath) so the rect is recomputed whenever Tippy queries it. While
touching this area, consider reusing the existing closeNoteNavigatorPopup helper
in the toggle-close path and the onSelect/onCancel callbacks instead of
duplicating destroy-and-null logic.
Comment on lines
+205
to
+207
| } else { | ||
| setMultiSelectedNoteIds(new Set()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
lastClickedNoteId isn't cleared when selection is dropped.
When selectedNoteId becomes falsy, multiSelectedNoteIds is reset but lastClickedNoteId keeps referencing the previously selected note, leaving a stale shift-click anchor.
🛠️ Proposed fix
} else {
setMultiSelectedNoteIds(new Set());
+ setLastClickedNoteId(null);
}📝 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.
Suggested change
| } else { | |
| setMultiSelectedNoteIds(new Set()); | |
| } | |
| } else { | |
| setMultiSelectedNoteIds(new Set()); | |
| setLastClickedNoteId(null); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/layout/Sidebar.tsx` around lines 205 - 207, The
selection-reset branch in Sidebar.tsx clears multiSelectedNoteIds but leaves
lastClickedNoteId pointing at the old note, so update the same else path that
handles selectedNoteId becoming falsy to also clear lastClickedNoteId. Use the
Sidebar selection state logic around setMultiSelectedNoteIds and
lastClickedNoteId so the shift-click anchor is reset whenever selection is
dropped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a global Cmd+G (Ctrl+G on Windows/Linux) shortcut that opens a searchable popup to jump to any note without modifying the current page. When a note is open the popup appears inline near the cursor; when no note is open it shows as a centered overlay.
This is based on the link editor (Cmd+K) that is used to insert a link to another wiki page - copying the same basic control and behaviour but making it available as a keyboard-shortcut friendly navigation interaction.
I've also updated the Sidebar so that the highlighting for the currently-selected page reflects the right page after a Cmd+G. This was arguably a separate issue as this feels like it was a bug affecting clicking on Wikilinks on pages, too.
Summary by CodeRabbit
New Features
Bug Fixes