feat: add opt-in auto-focus for new note titles#87
feat: add opt-in auto-focus for new note titles#87n00ki wants to merge 2 commits intoerictli:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds an optional setting to auto-focus new note titles, implements pending-title-focus tracking in NotesContext, and integrates editor logic to focus the top-level title when opening newly created notes. Also exposes the new setting in the general settings UI and persists it in backend types. Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant UI as Settings UI
participant Backend as Tauri/Storage
participant Notes as NotesContext
participant Editor
User->>UI: Toggle auto-focus setting
UI->>Backend: Persist setting
Backend-->>UI: Ack
User->>Notes: createNote(template)
Notes->>Notes: fetch settings (including autoFocusNewNoteTitle)
Notes->>Notes: set pendingTitleFocusRef (mode) if enabled
Notes-->>Editor: open note (noteId)
Editor->>Notes: consumePendingTitleFocus(noteId)
Notes-->>Editor: return mode | null
alt mode returned
Editor->>Editor: focusNewNoteTitle(mode) -> position cursor at H1
else no mode
Editor->>Editor: default focus behavior
end
Editor-->>User: Note displayed (cursor positioned)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/components/settings/GeneralSettingsSection.tsx`:
- Around line 121-140: Rapid toggles can spawn concurrent
invoke("update_settings") calls causing out-of-order persistence; modify
handleToggleAutoFocusNewNoteTitle to serialize updates by adding an in-flight
guard (e.g., isUpdatingAutoFocusRef) plus a pendingDesiredRef: if a call is
in-flight, set pendingDesiredRef.current = enabled and return (but keep
optimistic UI via setAutoFocusNewNoteTitle); when the in-flight call finishes
(success or error) clear isUpdatingAutoFocusRef and, if pendingDesiredRef
differs from the last-applied value, call the same update logic again to apply
the latest desired state; reference handleToggleAutoFocusNewNoteTitle,
autoFocusNewNoteTitle, setAutoFocusNewNoteTitle, and invoke("update_settings")
when making these changes.
In `@src/context/NotesContext.tsx`:
- Around line 178-186: The consumePendingTitleFocus function leaves
pendingTitleFocusRef.current set when the passed id doesn't match, allowing
stale focus to apply later; update consumePendingTitleFocus (and references to
pendingTitleFocusRef) so that if pendingTitleFocusRef.current is falsy or its id
!== id you explicitly set pendingTitleFocusRef.current = null and then return
null, otherwise capture mode, clear pendingTitleFocusRef.current = null and
return mode — ensuring the pending focus is always consumed/cleared as a strict
one-shot.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8d47178-8439-4881-9ccc-f86b8797aeb6
📒 Files selected for processing (5)
src-tauri/src/lib.rssrc/components/editor/Editor.tsxsrc/components/settings/GeneralSettingsSection.tsxsrc/context/NotesContext.tsxsrc/types/note.ts
| const handleToggleAutoFocusNewNoteTitle = async (enabled: boolean) => { | ||
| if (enabled === autoFocusNewNoteTitle) return; | ||
|
|
||
| const previous = autoFocusNewNoteTitle; | ||
| setAutoFocusNewNoteTitle(enabled); | ||
|
|
||
| try { | ||
| const settings = await invoke<Settings>("get_settings"); | ||
| await invoke("update_settings", { | ||
| newSettings: { | ||
| ...settings, | ||
| autoFocusNewNoteTitle: enabled ? true : undefined, | ||
| }, | ||
| }); | ||
| } catch (error) { | ||
| console.error("Failed to update title focus setting:", error); | ||
| setAutoFocusNewNoteTitle(previous); | ||
| toast.error("Failed to update setting"); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Prevent overlapping toggle writes to avoid out-of-order persistence.
Rapid On/Off clicks can run concurrent update_settings calls; slower earlier responses can overwrite the user’s latest choice.
💡 Proposed fix (serialize toggle updates with in-flight guard)
@@
- const [autoFocusNewNoteTitle, setAutoFocusNewNoteTitle] =
- useState<boolean>(false);
+ const [autoFocusNewNoteTitle, setAutoFocusNewNoteTitle] =
+ useState<boolean>(false);
+ const [isUpdatingAutoFocus, setIsUpdatingAutoFocus] = useState(false);
@@
const handleToggleAutoFocusNewNoteTitle = async (enabled: boolean) => {
- if (enabled === autoFocusNewNoteTitle) return;
+ if (isUpdatingAutoFocus || enabled === autoFocusNewNoteTitle) return;
const previous = autoFocusNewNoteTitle;
setAutoFocusNewNoteTitle(enabled);
+ setIsUpdatingAutoFocus(true);
try {
const settings = await invoke<Settings>("get_settings");
await invoke("update_settings", {
newSettings: {
...settings,
autoFocusNewNoteTitle: enabled ? true : undefined,
},
});
} catch (error) {
console.error("Failed to update title focus setting:", error);
setAutoFocusNewNoteTitle(previous);
toast.error("Failed to update setting");
+ } finally {
+ setIsUpdatingAutoFocus(false);
}
};
@@
<Button
onClick={() => handleToggleAutoFocusNewNoteTitle(false)}
variant={!autoFocusNewNoteTitle ? "primary" : "ghost"}
size="sm"
+ disabled={isUpdatingAutoFocus}
>
Off
</Button>
<Button
onClick={() => handleToggleAutoFocusNewNoteTitle(true)}
variant={autoFocusNewNoteTitle ? "primary" : "ghost"}
size="sm"
+ disabled={isUpdatingAutoFocus}
>
On
</Button>Also applies to: 577-590
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/settings/GeneralSettingsSection.tsx` around lines 121 - 140,
Rapid toggles can spawn concurrent invoke("update_settings") calls causing
out-of-order persistence; modify handleToggleAutoFocusNewNoteTitle to serialize
updates by adding an in-flight guard (e.g., isUpdatingAutoFocusRef) plus a
pendingDesiredRef: if a call is in-flight, set pendingDesiredRef.current =
enabled and return (but keep optimistic UI via setAutoFocusNewNoteTitle); when
the in-flight call finishes (success or error) clear isUpdatingAutoFocusRef and,
if pendingDesiredRef differs from the last-applied value, call the same update
logic again to apply the latest desired state; reference
handleToggleAutoFocusNewNoteTitle, autoFocusNewNoteTitle,
setAutoFocusNewNoteTitle, and invoke("update_settings") when making these
changes.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/settings/GeneralSettingsSection.tsx (1)
122-144: Good implementation with in-flight guard and optimistic UI.The handler correctly implements:
- Guard against concurrent updates (
isUpdatingAutoFocus)- Optimistic UI update with rollback on error
- Proper cleanup in
finallyblockOne minor observation: writing
undefinedinstead offalse(line 134) when disabled means the setting will be omitted from the persisted JSON rather than explicitly set tofalse. This works correctly since the read path uses?? false, but consider usingenableddirectly for consistency.💡 Optional: Use boolean value directly
await invoke("update_settings", { newSettings: { ...settings, - autoFocusNewNoteTitle: enabled ? true : undefined, + autoFocusNewNoteTitle: enabled, }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/GeneralSettingsSection.tsx` around lines 122 - 144, The toggle handler handleToggleAutoFocusNewNoteTitle currently writes undefined when disabling (using autoFocusNewNoteTitle: enabled ? true : undefined) which omits the key from persisted JSON; change the payload to set autoFocusNewNoteTitle: enabled (a boolean) when calling invoke("update_settings", { newSettings: { ...settings, autoFocusNewNoteTitle: enabled } }) so the setting is explicitly true/false and remains consistent with the read path and optimistic UI/rollback 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/components/settings/GeneralSettingsSection.tsx`:
- Around line 122-144: The toggle handler handleToggleAutoFocusNewNoteTitle
currently writes undefined when disabling (using autoFocusNewNoteTitle: enabled
? true : undefined) which omits the key from persisted JSON; change the payload
to set autoFocusNewNoteTitle: enabled (a boolean) when calling
invoke("update_settings", { newSettings: { ...settings, autoFocusNewNoteTitle:
enabled } }) so the setting is explicitly true/false and remains consistent with
the read path and optimistic UI/rollback logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ef628094-0b85-43d3-aade-593fe0571f29
📒 Files selected for processing (2)
src/components/settings/GeneralSettingsSection.tsxsrc/context/NotesContext.tsx
autoFocusNewNoteTitle, defaulting to off{date}/{counter}(quick append)NotesContextto avoid older note loads overriding current selection stateWhy
Auto-focusing title on note creation is useful for fast naming, but can feel intrusive as a default.
This keeps the workflow available for power users while preserving the current non-intrusive default experience.
Summary by CodeRabbit