Add macOS Menu Bar, Recordable Shortcuts, and Always-On-Top Scratchpad#21
Add macOS Menu Bar, Recordable Shortcuts, and Always-On-Top Scratchpad#21tiimmmoooo wants to merge 4 commits intoerictli:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive minimal editor window feature with global keyboard shortcut support. The changes include a new configurable shortcut system across the frontend and backend, window management for a separate minimal scratchpad editor, settings persistence for last-selected scratchpad and shortcuts, and macOS tray integration with global shortcut registration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OS as OS/Global Shortcut
participant App as App (React)
participant Backend as Tauri Backend
participant Settings as Settings Storage
User->>OS: Press Mod+Shift+M (minimal toggle)
OS->>App: Global shortcut event (global-toggle-minimal-editor)
App->>App: Rate-limit check & state validation
App->>Backend: toggle_minimal_editor()
Backend->>Backend: Check if minimal window exists
alt Minimal window not open
Backend->>Backend: get_or_create_minimal_scratchpad()
Backend->>Settings: Load/create minimal scratchpad
Backend->>Backend: Create minimal window
else Minimal window open
Backend->>Backend: Close/hide minimal window
end
Backend->>App: Return opened note id
App->>App: Update minimal mode state
App->>User: Show minimal editor or return focus
sequenceDiagram
participant User
participant Settings as Settings UI
participant Capture as Keyboard Capture
participant Theme as ThemeContext
participant Backend as Backend API
participant Storage as Settings Storage
User->>Settings: Click editable shortcut row
Settings->>Capture: Enable capture mode
User->>Capture: Press key combination (e.g., Cmd+B)
Capture->>Capture: Validate modifier requirements
alt Valid shortcut
Capture->>Theme: setShortcut(action, normalized)
Theme->>Backend: saveShortcutSettings()
Backend->>Storage: Persist shortcuts
Backend->>App: Re-register global shortcuts
Capture->>Settings: Display updated shortcut
else Invalid (missing modifier)
Capture->>User: Show validation error toast
end
User->>Settings: Click reset defaults
Settings->>Theme: resetShortcuts()
Theme->>Backend: saveShortcutSettings()
Backend->>Storage: Restore defaults
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Line 1: AGENTS.md currently contains only the text "CLAUDE.md" and appears to
be accidental; either remove AGENTS.md from the commit/PR or replace its
contents with the intended agent documentation; if it was meant as a
placeholder, update AGENTS.md to include the proper agent instructions or README
content (title, purpose, usage) instead of the lone "CLAUDE.md" string so the
repository no longer contains this meaningless file.
In `@src/components/command-palette/CommandPalette.tsx`:
- Around line 347-349: The useMemo that builds the command list in
CommandPalette.tsx omits shortcuts.openMinimalEditor from its dependency array,
so getShortcutDisplayText(shortcuts.openMinimalEditor) won't update when the
binding changes; update the dependency array for that useMemo to include
shortcuts.openMinimalEditor (alongside shortcuts.createNote,
shortcuts.toggleAlwaysOnTop, shortcuts.openSettings) so the memo recomputes and
the displayed label updates when the shortcut is remapped.
🧹 Nitpick comments (13)
src-tauri/capabilities/default.json (2)
4-5: Description is stale now that two windows share this capability.Line 4 says "Capability for the main window" but it now also covers the
minimalwindow. A small wording tweak (e.g., "Capability for application windows") would keep it accurate.✏️ Suggested fix
- "description": "Capability for the main window", + "description": "Capability for main and minimal windows",
5-25: Consider whether the minimal window needs the full permission set.The
minimalwindow now inherits every permission granted tomain, includingupdater:default,dialog:allow-open,opener:allow-reveal-item-in-dir, and broadfsaccess. If the minimal scratchpad only needs to read/write notes, you could define a narrower capability for it, following the principle of least privilege. Not blocking, but worth considering. As per coding guidelines, "Add new permissions to src-tauri/capabilities/default.json following Tauri v2 capability-based permissions."src/App.css (1)
116-127: The dark-mode override is identical to the base rule and can be removed.Lines 123–127 (
html.dark .minimal-scratchpad-shell) declare the exact sameborder,background, andbox-shadowas lines 116–121. Since both reference the same CSS custom properties (which already switch values via the.darkclass on:root), the dark-mode block is redundant.Additionally, Stylelint flags the
rgba(255, 255, 255, 0.04)on lines 120 and 126 — modern CSS notation should usergb(255 255 255 / 4%).♻️ Proposed fix
.minimal-scratchpad-shell { border-radius: 1.1rem; border: 1px solid var(--color-border); background: var(--color-bg); - box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.04); + box-shadow: inset 0 1px 0 rgb(255 255 255 / 4%); } -html.dark .minimal-scratchpad-shell { - border: 1px solid var(--color-border); - background: var(--color-bg); - box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.04); -}src/context/ThemeContext.tsx (2)
260-285: No duplicate shortcut conflict detection.
setShortcutvalidates that the shortcut is well-formed and has the required modifiers, but it doesn't check whether the same shortcut string is already assigned to a different action. If a user binds the same key combo to two actions, both handlers will fire on keypress. Consider checking for conflicts and either rejecting the assignment or unassigning the conflicting action.♻️ Sketch of a conflict check
const setShortcut = useCallback( (action: ShortcutAction, shortcut: string) => { const normalized = normalizeShortcut(shortcut); if (!normalized) return false; const parsed = parseShortcut(normalized); if (!parsed) return false; const hasAnyModifier = parsed.mod || parsed.alt || parsed.shift; const allowWithoutCmdCtrl = action === "navigateNoteUp" || action === "navigateNoteDown" || action === "openMinimalEditor"; if (!allowWithoutCmdCtrl && !parsed.mod) return false; if (action === "openMinimalEditor" && !hasAnyModifier) return false; + // Check for conflicts with other actions + const conflicting = Object.entries(shortcuts).find( + ([key, value]) => key !== action && normalizeShortcut(value) === normalized, + ); + if (conflicting) return false; + setShortcuts((prev) => { const updated = { ...prev, [action]: normalized }; saveShortcutSettings(updated); return updated; }); return true; }, - [saveShortcutSettings] + [saveShortcutSettings, shortcuts] );
244-258: Read-modify-write race insaveShortcutSettings.
getSettings()→updateSettings({...settings, shortcuts})is a non-atomic read-modify-write. If a theme or font save runs concurrently, one write can clobber the other. The same pattern exists forsaveThemeSettingsandsaveFontSettings, so this is a pre-existing issue, but it grows more likely now that three independent save paths share the same settings object. Not blocking, but worth noting for future consolidation (e.g., a debounced single-writer or optimistic merge).src/App.tsx (2)
600-606:getCurrentWindow()is called on every render ofApp.
isMinimalWindowis computed in the function body, sogetCurrentWindow()runs each timeAppre-renders. The result never changes (a window's label is constant), so this could be hoisted to module scope or memoized. It's unlikely to cause a real performance issue sinceAppre-renders infrequently, but it's slightly cleaner as a constant.♻️ Hoist to module scope
+let isMinimalWindow = false; +try { + isMinimalWindow = getCurrentWindow().label === "minimal"; +} catch { + // Running without Tauri (e.g. plain Vite dev server). +} + function App() { - let isMinimalWindow = false; - try { - isMinimalWindow = getCurrentWindow().label === "minimal"; - } catch { - // Running without Tauri (e.g. plain Vite dev server). - } - // Add platform class for OS-specific styling ...
146-152: PersistinglastSelectedScratchpadIdon every note selection.This fires on every
selectedNoteIdchange in the main window, which means every note switch triggers a backend write. This is fine if the backend write is cheap (just updating a JSON settings file), but could be noisy. Consider debouncing this if it proves to be a performance concern — the coding guidelines specify debouncing for "auto-save (300ms)." For now it's acceptable since note switches are infrequent and user-initiated.src/components/settings/ShortcutsSettingsSection.tsx (1)
159-161: Potential duplicate React key inShortcutKeys.Using
key={key}means that if a shortcut display ever contains duplicate strings (e.g., two identical modifier symbols), React will warn. Consider using the index as key or a composite key.Proposed fix
- {keys.map((key) => ( - <KeyboardKey key={key} keyLabel={key} /> + {keys.map((key, i) => ( + <KeyboardKey key={`${key}-${i}`} keyLabel={key} /> ))}src/components/editor/Editor.tsx (1)
1225-1235: Fragile sidebar detection using CSS class substring match.
target.closest('[class*="sidebar"]')relies on a specific class naming convention. If class names are changed, minified, or use a different naming pattern, this check will silently break, causing find-in-note to trigger while focused in the sidebar.Consider using a
data-*attribute instead for a more robust contract.Example
In the sidebar container element, add:
<div data-region="sidebar" ...>Then in the check:
- if (target.closest('[class*="sidebar"]')) { + if (target.closest('[data-region="sidebar"]')) { return; }src-tauri/src/lib.rs (3)
638-662:unregister_all()removes all global shortcuts indiscriminately.
register_global_minimal_shortcutcallsglobal_shortcut.unregister_all()on Line 641 before registering the minimal editor shortcut. If additional global shortcuts are registered in the future, this will silently remove them. Consider tracking and unregistering only the previously-registered minimal shortcut instead.Sketch: track previous shortcut and unregister selectively
+// Store the last registered shortcut string in AppState so we can unregister just that one. +pub last_registered_shortcut: Mutex<Option<String>>,Then in
register_global_minimal_shortcut:- global_shortcut - .unregister_all() - .map_err(|e| e.to_string())?; + // Unregister only the previous shortcut if one was registered + if let Some(prev) = state.last_registered_shortcut.lock().expect("mutex").take() { + let _ = global_shortcut.unregister(prev.as_str()); + }
2173-2183: Redundant double pattern match onCloseRequestedevent.The outer
matches!check and innerif letboth match the same variant. You can simplify to a singleif let.Proposed simplification
.on_window_event(|window, event| { #[cfg(target_os = "macos")] - if window.label() == "main" - && matches!(event, tauri::WindowEvent::CloseRequested { .. }) { - if let tauri::WindowEvent::CloseRequested { api, .. } = event { + if window.label() == "main" { + if let tauri::WindowEvent::CloseRequested { api, .. } = event { api.prevent_close(); let _ = window.hide(); } } })
948-959: Settings write-then-read pattern has a narrow race window.Between releasing the write lock (line 955) and acquiring the read lock (line 957), another thread could modify
settings. In a single-user desktop app this is unlikely, but the pattern could be simplified to save within the write lock scope.Suggested pattern: save while still holding data
let mut settings_changed = false; + let settings_snapshot; if let Some((ref old_id_str, _)) = old_id { let mut settings = state.settings.write().expect("settings write lock"); if settings.last_selected_scratchpad_id.as_deref() == Some(old_id_str) { settings.last_selected_scratchpad_id = Some(final_id.clone()); settings_changed = true; } + settings_snapshot = settings.clone(); + } else { + settings_snapshot = state.settings.read().expect("settings read lock").clone(); } if settings_changed { - let settings = state.settings.read().expect("settings read lock"); - save_settings(&folder, &settings).map_err(|e| e.to_string())?; + save_settings(&folder, &settings_snapshot).map_err(|e| e.to_string())?; }src/lib/shortcuts.ts (1)
84-99: Consider handling theDeadkey event for international keyboards.On some keyboard layouts (e.g., German, French), certain key combinations produce
event.key === "Dead"for dead keys (accents).normalizeEventKeywould pass "Dead" through tonormalizeKeyTokenand produce "Dead" as a valid key. This could allow recording a shortcut with "Dead" as the key, which won't match on subsequent presses since the actual composed character will differ.This is an edge case that may not need immediate attention but is worth noting if international keyboard support matters.
| @@ -0,0 +1 @@ | |||
| CLAUDE.md No newline at end of file | |||
There was a problem hiding this comment.
This file appears to be committed by mistake.
AGENTS.md contains only the text "CLAUDE.md" with no meaningful content. Was this intentionally included in the PR, or is it an artifact? If it's meant to be an agent/AI instruction file, it likely needs actual content; otherwise, consider removing it.
🤖 Prompt for AI Agents
In `@AGENTS.md` at line 1, AGENTS.md currently contains only the text "CLAUDE.md"
and appears to be accidental; either remove AGENTS.md from the commit/PR or
replace its contents with the intended agent documentation; if it was meant as a
placeholder, update AGENTS.md to include the proper agent instructions or README
content (title, purpose, usage) instead of the lone "CLAUDE.md" string so the
repository no longer contains this meaningless file.
| shortcuts.createNote, | ||
| shortcuts.toggleAlwaysOnTop, | ||
| shortcuts.openSettings, |
There was a problem hiding this comment.
Missing shortcuts.openMinimalEditor in useMemo dependency array.
Line 130 uses getShortcutDisplayText(shortcuts.openMinimalEditor) to render the shortcut label, but shortcuts.openMinimalEditor is not listed in the dependency array. If the user remaps this shortcut, the command palette label won't update.
Proposed fix
shortcuts.createNote,
shortcuts.toggleAlwaysOnTop,
+ shortcuts.openMinimalEditor,
shortcuts.openSettings,📝 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.
| shortcuts.createNote, | |
| shortcuts.toggleAlwaysOnTop, | |
| shortcuts.openSettings, | |
| shortcuts.createNote, | |
| shortcuts.toggleAlwaysOnTop, | |
| shortcuts.openMinimalEditor, | |
| shortcuts.openSettings, |
🤖 Prompt for AI Agents
In `@src/components/command-palette/CommandPalette.tsx` around lines 347 - 349,
The useMemo that builds the command list in CommandPalette.tsx omits
shortcuts.openMinimalEditor from its dependency array, so
getShortcutDisplayText(shortcuts.openMinimalEditor) won't update when the
binding changes; update the dependency array for that useMemo to include
shortcuts.openMinimalEditor (alongside shortcuts.createNote,
shortcuts.toggleAlwaysOnTop, shortcuts.openSettings) so the memo recomputes and
the displayed label updates when the shortcut is remapped.
|
@tiimmmoooo lots of good ideas here. May need a bit of time to review this, will try to look over the weekend. |
|
Update: this is a big one so haven't gotten to it yet. I'm trying to keep the app slim so I may need to look at the individual features and implement some of them individually. Will credit you if i do. |
2 participants
Features Added
Why?
Felt it is a massive QoL change
Summary by CodeRabbit
Release Notes
New Features
Chores