feat: use a commands for a user interactions - #195
Merged
vitonsky merged 85 commits intoMar 8, 2026
Merged
Conversation
katsyuta
force-pushed
the
116-use-commands-for-user-interactions
branch
from
January 4, 2026 20:52
ad27f4a to
e071a03
Compare
katsyuta
changed the title
katsyuta
marked this pull request as ready for review
January 6, 2026 21:47
katsyuta
force-pushed
the
116-use-commands-for-user-interactions
branch
2 times, most recently
from
January 12, 2026 23:24
ed2b7f8 to
ea535f1
Compare
vitonsky
marked this pull request as draft
January 13, 2026 16:46
katsyuta
force-pushed
the
116-use-commands-for-user-interactions
branch
from
January 13, 2026 22:56
f3fc234 to
ff01ae2
Compare
katsyuta
marked this pull request as ready for review
January 13, 2026 23:05
katsyuta
force-pushed
the
116-use-commands-for-user-interactions
branch
from
January 23, 2026 21:06
7669598 to
e35ce32
Compare
katsyuta
commented
Jan 24, 2026
vitonsky
requested changes
Jan 27, 2026
katsyuta
force-pushed
the
116-use-commands-for-user-interactions
branch
3 times, most recently
from
February 2, 2026 13:25
e312f8d to
f485353
Compare
katsyuta
force-pushed
the
116-use-commands-for-user-interactions
branch
from
February 14, 2026 12:16
49eaebf to
0d6ca6b
Compare
vitonsky
requested changes
Feb 19, 2026
katsyuta
force-pushed
the
116-use-commands-for-user-interactions
branch
from
February 19, 2026 17:53
2715bfe to
c7f0d07
Compare
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request refactors the application to use a command-dispatch architecture for user interactions. It replaces direct callbacks and state updates with centralized command handlers, introduces a dedicated shortcuts management system, and migrates note-related operations (archiving, bookmarking, deleting, exporting, copying links, duplicating) to command-based flows across the editor, menus, and context menus. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as NoteEditor/Menu
participant Command as Command System
participant Handler as Note Handlers
participant Registry as Notes Registry
User->>UI: Click archive/bookmark/delete button
UI->>Command: dispatch GLOBAL_COMMAND with noteId
Command->>Handler: execute registered handler
Handler->>Registry: fetch note, update state
Handler->>Handler: emit NOTE_UPDATED/NOTES_UPDATED
Handler->>Handler: log telemetry
Handler-->>UI: (async, state update via event)
UI-->>User: UI reflects updated note state
sequenceDiagram
participant User
participant Keyboard as Keyboard Input
participant Shortcuts as Shortcuts System
participant Command as Command System
participant Handlers as Workspace Handlers
User->>Keyboard: Press shortcut key (e.g., Cmd+D)
Keyboard->>Shortcuts: useShortcutsBinding detects keystroke
Shortcuts->>Command: dispatch SHORTCUT_PRESSED with shortcut enum
Command->>Handlers: useShortcutCallback matches & calls handler
Handlers->>Handlers: execute associated GLOBAL_COMMAND
Handlers-->>User: action completed (delete, archive, etc.)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/App/Profile/index.tsx`:
- Line 162: The current useCommandCallback handler calls db.sync() without
awaiting or handling rejections, causing potential unhandled promise rejections;
update the callback registered with
useCommandCallback(GLOBAL_COMMANDS.SYNC_DATABASE, ...) to call db.sync() and
attach explicit error handling (e.g., .catch or async/try-catch) that logs the
error and handles failure appropriately instead of leaving the Promise
unhandled—ensure you reference the useCommandCallback registration and the
db.sync() call so the fix is applied to the exact listener.
In `@src/features/App/Workspace/useWorkspaceShortcutsHandlers.ts`:
- Around line 76-87: The current shortcut handler for
Shortcuts.DELETE_CURRENT_NOTE uses deletionConfig.permanentDeletion to decide
deletion mode, so it doesn't support a distinct "permanently delete" shortcut;
update this by adding a new shortcut handler (e.g.,
Shortcuts.DELETE_CURRENT_NOTE_PERMANENT) alongside the existing
useShortcutCallback for Shortcuts.DELETE_CURRENT_NOTE and call
command(GLOBAL_COMMANDS.DELETE_NOTE, { noteId: activeNoteId, permanently: true
}) in that new handler, while leaving the existing handler to use
deletionConfig.permanentDeletion; reference deletionConfig, useShortcutCallback,
Shortcuts.DELETE_CURRENT_NOTE, the new Shortcuts constant name you choose,
GLOBAL_COMMANDS.DELETE_NOTE, and activeNoteId when making the changes.
In `@src/features/NoteEditor/index.tsx`:
- Around line 213-221: The TOGGLE_NOTE_HISTORY_PANEL handler registered with
useWorkspaceCommandCallback (GLOBAL_COMMANDS.TOGGLE_NOTE_HISTORY_PANEL)
currently ignores the command payload and toggles the panel for every editor;
update the callback to inspect the incoming payload (e.g., payload.noteId) and
only call setSidePanel to toggle NoteSidebarTabs.HISTORY when payload.noteId
matches this editor's noteId (or when payload is undefined for broadcast
behavior), leaving other editors untouched; ensure the check uses the same
noteId identifier available in this component and preserve the existing isActive
gating.
In `@src/features/NoteEditor/NoteMenu.tsx`:
- Around line 51-56: Several menu entries in NoteMenu.tsx (the MenuItem elements
around the shown FaBell/HStack block and the groups at ranges 72-95 and 107-118)
are placeholders that look actionable but do nothing; mark those MenuItem
instances as disabled (e.g., add the isDisabled prop and appropriate
aria-disabled/title text) or replace them with non-interactive elements so they
cannot be clicked until their handlers are implemented, and ensure any visual
affordance (opacity/tooltip) communicates they are disabled.
- Around line 35-37: The icon-only menu trigger (MenuButton in NoteMenu.tsx)
lacks an accessible name; add an aria-label (or aria-labelledby to a visually
hidden label) to the MenuButton used as the Button (the MenuButton as={Button}
variant="ghost" size="sm") so assistive tech gets a clear name (e.g., "More
options" or similar); update the MenuButton props to include aria-label="More
options" or hook it up to a hidden span and ensure the label text is localized
if applicable.
In `@src/hooks/notes/useNoteCommandHandlers.ts`:
- Around line 128-130: The warning message in useNoteCommandHandlers (inside the
handler where it checks if (!sourceNote)) logs the wrong variable; change the
console.warn call to interpolate the requested note id (noteId) instead of
sourceNote so the message shows the actual missing note id; locate the check
using sourceNote and update the log string to reference noteId and include clear
context (e.g., "Not found note with id ${noteId}") to avoid misleading output.
- Line 119: Remove the plaintext markdown-link logging in
useNoteCommandHandlers: locate the console.log(`Copy markdown link
${markdownLink}`) call inside the handler in useNoteCommandHandlers and delete
it (or replace with a non-sensitive diagnostic such as a static debug flag or
count). Ensure no note-derived title or id is emitted to runtime logs; if you
need telemetry, send an anonymized event (e.g., success boolean) instead of the
markdownLink string.
- Around line 93-102: The handler registered with useWorkspaceCommandCallback
for GLOBAL_COMMANDS.EXPORT_NOTE calls notesExport.exportNote even when
notes.getById([noteId]) returns undefined; update the callback to check the
retrieved note (from notes.getById) before calling notesExport.exportNote: if
note is missing, bail out early (return or log/report an error) and avoid
calling buildFileName with note?.content; otherwise proceed to call
notesExport.exportNote with the safe filename built from workspaceData?.name and
note.content.title (trimmed and truncated) so you never dereference a missing
note.
In `@src/hooks/shortcuts/index.ts`:
- Around line 33-35: The shortcuts map is missing a binding for the
permanent-delete action; add a key 'CmdOrCtrl+Shift+Delete' mapped to the
appropriate enum value (e.g., Shortcuts.PERMANENT_DELETE) alongside the existing
'CmdOrCtrl+Delete' and 'CmdOrCtrl+Shift+R' entries in
src/hooks/shortcuts/index.ts, and if the Shortcuts enum/type lacks a
PERMANENT_DELETE member add it to the Shortcuts declaration so the action can be
uniquely triggered.
In `@src/hooks/shortcuts/useShortcutsBinding.ts`:
- Around line 35-45: The hotkeys handler created in the loop over
normalizedShortcuts (where hotkeys(...) calls runCommand with
GLOBAL_COMMANDS.SHORTCUT_PRESSED) must suppress native browser actions; update
the handler to accept the event parameter and call event.preventDefault() (or
return false) before invoking runCommand so shortcuts like Cmd/Ctrl+F and
Cmd/Ctrl+S don't trigger default browser behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9fd5224f-5c23-4a29-a9bb-904dec8dca04
📒 Files selected for processing (21)
src/features/App/Profile/index.tsxsrc/features/App/Profile/useVaultShortcutsHandlers.tssrc/features/App/Settings/sections/HotKeysSettings.tsxsrc/features/App/Workspace/useActiveNoteHistoryButton.tsxsrc/features/App/Workspace/useWorkspaceShortcutsHandlers.tssrc/features/MainScreen/NotesListPanel/NotesList.tsxsrc/features/MainScreen/NotesListPanel/index.tsxsrc/features/MainScreen/index.tsxsrc/features/NoteEditor/NoteMenu.tsxsrc/features/NoteEditor/index.tsxsrc/features/NotesContainer/NoteContextMenu/useNoteContextMenu.tssrc/features/NotesContainer/OpenedNotesPanel.tsxsrc/features/NotesContainer/index.tsxsrc/hooks/commands/index.tssrc/hooks/commands/shortcuts/index.tssrc/hooks/commands/useCommandCallback.tssrc/hooks/notes/useNoteCommandHandlers.tssrc/hooks/notes/useNotesShortcutActions.tssrc/hooks/shortcuts/index.tssrc/hooks/shortcuts/useShortcutCallback.tssrc/hooks/shortcuts/useShortcutsBinding.ts
💤 Files with no reviewable changes (1)
- src/hooks/commands/shortcuts/index.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/hooks/notes/useNoteCommandHandlers.ts (2)
32-36: Consolidate duplicateuseNotesRegistry()calls.Both
notes(line 32) andnotesRegistry(line 36) call the same hook, creating redundant references. Consider using a single variable throughout the hook for consistency.♻️ Suggested consolidation
const telemetry = useTelemetryTracker(); const notes = useNotesRegistry(); const tagsRegistry = useTagsRegistry(); const noteActions = useNoteActions(); - const notesRegistry = useNotesRegistry();Then update the usages in
TOGGLE_NOTE_ARCHIVEandTOGGLE_NOTE_BOOKMARKhandlers to usenotesinstead ofnotesRegistry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/notes/useNoteCommandHandlers.ts` around lines 32 - 36, There are duplicate calls to useNotesRegistry(): the variables notes and notesRegistry both reference the same hook; remove the redundant call by keeping a single variable (e.g., notes) returned from useNotesRegistry() and delete the other declaration, then update all usages within this file—specifically inside the TOGGLE_NOTE_ARCHIVE and TOGGLE_NOTE_BOOKMARK handlers—to reference notes instead of notesRegistry (ensure any other references in useNoteCommandHandlers, noteActions or related handlers now use the consolidated variable).
93-104: Minor: Unnecessary optional chaining after null guard.At line 101,
note?.content.titleuses optional chaining even thoughnoteis confirmed to exist at line 95. This is not a bug but a minor inconsistency.♻️ Suggested fix
await notesExport.exportNote( noteId, buildFileName( workspaceData?.name, - note?.content.title.trim().slice(0, 50).trim() || `note_${noteId}`, + note.content.title.trim().slice(0, 50).trim() || `note_${noteId}`, ), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/notes/useNoteCommandHandlers.ts` around lines 93 - 104, The optional chaining on note (note?.content.title) is redundant because note is already null-guarded earlier; update the EXPORT_NOTE handler (useWorkspaceCommandCallback for GLOBAL_COMMANDS.EXPORT_NOTE) to use note.content.title instead of note?.content.title when building the filename (references: notes.getById, notesExport.exportNote, buildFileName, workspaceData, note.content.title) to remove the unnecessary optional chaining.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/hooks/notes/useNoteCommandHandlers.ts`:
- Around line 32-36: There are duplicate calls to useNotesRegistry(): the
variables notes and notesRegistry both reference the same hook; remove the
redundant call by keeping a single variable (e.g., notes) returned from
useNotesRegistry() and delete the other declaration, then update all usages
within this file—specifically inside the TOGGLE_NOTE_ARCHIVE and
TOGGLE_NOTE_BOOKMARK handlers—to reference notes instead of notesRegistry
(ensure any other references in useNoteCommandHandlers, noteActions or related
handlers now use the consolidated variable).
- Around line 93-104: The optional chaining on note (note?.content.title) is
redundant because note is already null-guarded earlier; update the EXPORT_NOTE
handler (useWorkspaceCommandCallback for GLOBAL_COMMANDS.EXPORT_NOTE) to use
note.content.title instead of note?.content.title when building the filename
(references: notes.getById, notesExport.exportNote, buildFileName,
workspaceData, note.content.title) to remove the unnecessary optional chaining.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa08c910-547e-48dc-b347-e0e55b7d5ae9
📒 Files selected for processing (1)
src/hooks/notes/useNoteCommandHandlers.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/features/NoteEditor/NoteMenu.tsx (1)
35-37:⚠️ Potential issue | 🟡 MinorThe
titleattribute does not provide an accessible name for screen readers.While
title="Note options"was added, thetitleattribute is not reliably announced by screen readers and doesn't serve as an accessible name. The previous review suggested addingaria-label, which is the correct approach for icon-only buttons.♿ Suggested fix
- <MenuButton as={Button} variant="ghost" size="sm" title="Note options"> + <MenuButton + as={Button} + variant="ghost" + size="sm" + title="Note options" + aria-label="Note options" + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/NoteEditor/NoteMenu.tsx` around lines 35 - 37, The MenuButton icon-only control in NoteMenu.tsx (the MenuButton as={Button} rendering FaEllipsis) lacks a proper accessible name; add an aria-label (e.g., aria-label="Note options") to the MenuButton component so screen readers get a reliable name (you can keep or remove the title attribute but ensure aria-label is present on the MenuButton/Button element that renders the FaEllipsis icon).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/features/NoteEditor/NoteMenu.tsx`:
- Around line 35-37: The MenuButton icon-only control in NoteMenu.tsx (the
MenuButton as={Button} rendering FaEllipsis) lacks a proper accessible name; add
an aria-label (e.g., aria-label="Note options") to the MenuButton component so
screen readers get a reliable name (you can keep or remove the title attribute
but ensure aria-label is present on the MenuButton/Button element that renders
the FaEllipsis icon).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7ab6815-1186-4e10-89e6-46f81a876fb4
📒 Files selected for processing (1)
src/features/NoteEditor/NoteMenu.tsx
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks/notes/useNoteCommandHandlers.ts (1)
92-186: Consider extracting a shared note-fetch guard helper.Several handlers repeat
getById+ missing-note logging/return logic. A small helper would reduce duplication and keep guard behavior uniform.♻️ Refactor sketch
+const getNoteOrWarn = async (noteId: string) => { + const [note] = await notes.getById([noteId]); + if (!note) { + console.warn(`Not found note with id ${noteId}`); + return null; + } + return note; +}; + useWorkspaceCommandCallback(GLOBAL_COMMANDS.EXPORT_NOTE, async ({ noteId }) => { - const [note] = await notes.getById([noteId]); - if (!note) return; + const note = await getNoteOrWarn(noteId); + if (!note) return; // ... }); useWorkspaceCommandCallback( GLOBAL_COMMANDS.TOGGLE_NOTE_ARCHIVE, async ({ noteId }) => { - const [note] = await notes.getById([noteId]); - if (!note) { - console.warn(`Not found note with id ${noteId}`); - return; - } + const note = await getNoteOrWarn(noteId); + if (!note) return; // ... }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/notes/useNoteCommandHandlers.ts` around lines 92 - 186, Extract a small helper (e.g., fetchNoteOrWarn or getNoteOrLog) that wraps notes.getById(noteId) and centralizes the missing-note behavior (log a consistent warning/error and return null/undefined), then replace the repeated patterns inside the useWorkspaceCommandCallback handlers for GLOBAL_COMMANDS.EXPORT_NOTE, COPY_NOTE_MARKDOWN_LINK, DUPLICATE_NOTE, TOGGLE_NOTE_ARCHIVE and TOGGLE_NOTE_BOOKMARK to call this helper and early-return when it yields no note; ensure the helper preserves the original logging levels/messages used in the current handlers (console.error/console.warn) and update consumers to use the returned note object for their subsequent logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/hooks/notes/useNoteCommandHandlers.ts`:
- Around line 92-186: Extract a small helper (e.g., fetchNoteOrWarn or
getNoteOrLog) that wraps notes.getById(noteId) and centralizes the missing-note
behavior (log a consistent warning/error and return null/undefined), then
replace the repeated patterns inside the useWorkspaceCommandCallback handlers
for GLOBAL_COMMANDS.EXPORT_NOTE, COPY_NOTE_MARKDOWN_LINK, DUPLICATE_NOTE,
TOGGLE_NOTE_ARCHIVE and TOGGLE_NOTE_BOOKMARK to call this helper and
early-return when it yields no note; ensure the helper preserves the original
logging levels/messages used in the current handlers
(console.error/console.warn) and update consumers to use the returned note
object for their subsequent logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24e365f5-9227-4241-b0a5-136bdf43237f
📒 Files selected for processing (1)
src/hooks/notes/useNoteCommandHandlers.ts
katsyuta
force-pushed
the
116-use-commands-for-user-interactions
branch
from
March 7, 2026 23:32
5244e23 to
097dd85
Compare
vitonsky
reviewed
Mar 8, 2026
vitonsky
approved these changes
Mar 8, 2026
vitonsky
changed the title
3 tasks
This was referenced Apr 8, 2026
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
2 participants
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.
Closes #116
Changes:
CmdOrCtrl+Shift+A: toggle current note archiveCmdOrCtrl+Shift+B: toggle current note bookmarkCmdOrCtrl+Delete: delete current note to binCmdOrCtrl+Shift+R: restore current note from binCmdOrCtrl+S: save changes to dbCmdOrCtrl+H: open note historyCmdOrCtrl+F: focus to search inputCmdOrCtrl+.: open settingSummary by CodeRabbit
New Features
Tests