Skip to content

Commit 8eeb786

Browse files
Refactor effect-driven events into imperative handlers
Focus the note-row button directly in handleNoteRowPointerDown instead of setting a ref flag and restoring focus in a useEffect. Introduce useCommandRequest/useCommandRequestId to replace the requestId + lastHandledRef + useEffect pattern at seven call sites. Handlers returning false defer the mark so retry-on-data-load cases still work.
1 parent 9b73ecb commit 8eeb786

8 files changed

Lines changed: 119 additions & 193 deletions

File tree

app/src/features/editor/hooks/use-find-bar.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
} from "react";
88

99
import { type NoteEditorHandle } from "@/features/editor/note-editor";
10+
import { useCommandRequestId } from "@/shared/hooks/use-command-request";
1011
import { useShellCommandStore } from "@/shared/stores/use-shell-command-store";
1112
import {
1213
isEditorFindShortcut,
@@ -35,7 +36,6 @@ export function useFindBar({
3536
);
3637
const findInputRef = useRef<HTMLInputElement | null>(null);
3738
const lastActiveNoteIdRef = useRef(noteId);
38-
const lastHandledEditorFindRequestIdRef = useRef(0);
3939
const hasEditorFindQuery = findOpen && findQuery.trim().length > 0;
4040

4141
const activeEditorSearch = resolveActiveEditorSearch({
@@ -145,17 +145,9 @@ export function useFindBar({
145145
};
146146
}, []);
147147

148-
useEffect(() => {
149-
if (
150-
editorFindRequestId === 0 ||
151-
lastHandledEditorFindRequestIdRef.current === editorFindRequestId
152-
) {
153-
return;
154-
}
155-
156-
lastHandledEditorFindRequestIdRef.current = editorFindRequestId;
148+
useCommandRequestId(editorFindRequestId, () => {
157149
openFind();
158-
}, [editorFindRequestId, openFind]);
150+
});
159151

160152
return {
161153
findOpen,

app/src/features/editor/ui/editor-pane.tsx

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
useShowEditorToolbar,
1919
useUIActions,
2020
} from "@/shared/stores/use-ui-store";
21+
import { useCommandRequest } from "@/shared/hooks/use-command-request";
2122
import { useShellCommandStore } from "@/shared/stores/use-shell-command-store";
2223
import { useShellNavigationStore } from "@/shared/stores/use-shell-navigation-store";
2324
import cometLogo from "@/assets/comet.svg";
@@ -199,7 +200,6 @@ export function EditorPane({
199200
setToolbarContainer(node);
200201
};
201202
const editorLoadKey = noteId ? (editorKey ?? noteId) : null;
202-
const lastHandledFocusEditorRequestIdRef = useRef(0);
203203

204204
const openEditorMenu = async (position: LogicalPosition) => {
205205
if (!noteId) return;
@@ -278,37 +278,22 @@ export function EditorPane({
278278
window.removeEventListener("keydown", handleGlobalHistoryKeyDown);
279279
}, []);
280280

281-
const handleFocusEditor = useEffectEvent(
282-
(scrollTo: "preserve" | "top" = "preserve") => {
283-
if (!noteId) {
284-
return;
285-
}
286-
287-
setFocusedPane("editor");
288-
requestAnimationFrame(() => {
289-
if (scrollTo === "top") {
290-
editorRef.current?.focusAtStart();
291-
scrollContainerRef.current?.scrollTo({ top: 0 });
292-
return;
293-
}
294-
295-
editorRef.current?.focus();
296-
});
297-
},
298-
);
299-
300-
useEffect(() => {
301-
if (
302-
!focusEditorRequest ||
303-
lastHandledFocusEditorRequestIdRef.current ===
304-
focusEditorRequest.requestId
305-
) {
281+
useCommandRequest(focusEditorRequest, (request) => {
282+
if (!noteId) {
306283
return;
307284
}
308285

309-
lastHandledFocusEditorRequestIdRef.current = focusEditorRequest.requestId;
310-
handleFocusEditor(focusEditorRequest.scrollTo);
311-
}, [focusEditorRequest, handleFocusEditor]);
286+
setFocusedPane("editor");
287+
requestAnimationFrame(() => {
288+
if (request.scrollTo === "top") {
289+
editorRef.current?.focusAtStart();
290+
scrollContainerRef.current?.scrollTo({ top: 0 });
291+
return;
292+
}
293+
294+
editorRef.current?.focus();
295+
});
296+
});
312297

313298
const headerActions =
314299
noteId !== null ? (

app/src/features/notes-pane/ui/note-row.tsx

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ export type NoteRowProps = {
3434
searchWords: string[];
3535
selectedNoteId: string | null;
3636
setSlideInNoteId(noteId: string | null): void;
37-
setShouldRestoreSelectedRowFocus(): void;
3837
shouldSkipAnimation: boolean;
3938
};
4039

@@ -53,7 +52,6 @@ export function NoteRow({
5352
searchWords,
5453
selectedNoteId,
5554
setSlideInNoteId,
56-
setShouldRestoreSelectedRowFocus,
5755
shouldSkipAnimation,
5856
}: NoteRowProps) {
5957
const isActive = note.id === selectedNoteId;
@@ -124,10 +122,7 @@ export function NoteRow({
124122
setFocusedPane("sidebar");
125123
}
126124
}}
127-
onPointerDown={(event) => {
128-
handleNoteRowPointerDown(event);
129-
setShouldRestoreSelectedRowFocus();
130-
}}
125+
onPointerDown={handleNoteRowPointerDown}
131126
onMouseDown={(event) => {
132127
if (event.button === 2) {
133128
event.preventDefault();

app/src/features/notes-pane/ui/notes-pane-utils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ export function handleNoteRowPointerDown(
117117

118118
event.preventDefault();
119119
window.getSelection()?.removeAllRanges();
120+
event.currentTarget.focus({ preventScroll: true });
120121
}
121122

122123
export function focusSelectedNoteRow(root?: ParentNode | null) {

app/src/features/notes-pane/ui/notes-pane.tsx

Lines changed: 13 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import { useEffect, useRef, useState, type MouseEvent } from "react";
44
import { useInView } from "react-intersection-observer";
55

66
import { Button } from "@/shared/ui/button";
7+
import {
8+
useCommandRequest,
9+
useCommandRequestId,
10+
} from "@/shared/hooks/use-command-request";
711
import { searchWordsFromQuery } from "@/shared/lib/search";
812
import {
913
type NoteListNavigationDirection,
@@ -155,10 +159,7 @@ export function NotesPane({
155159
const pendingNotesPaneSelectionRef = useRef<"first" | "selected" | null>(
156160
null,
157161
);
158-
const lastHandledFocusNotesPaneRequestIdRef = useRef(0);
159-
const lastHandledFocusNotesSearchRequestIdRef = useRef(0);
160162
const noteRowRefs = useRef(new Map<string, HTMLButtonElement | null>());
161-
const shouldRestoreSelectedRowFocusRef = useRef(false);
162163
const [isSearchFocused, setIsSearchFocused] = useState(false);
163164
const scrollContainerRef = useRef<HTMLDivElement | null>(null);
164165
const { ref: loadMoreRef, inView } = useInView({
@@ -245,34 +246,15 @@ export function NotesPane({
245246
searchInputRef.current?.select();
246247
}, [isSearchOpen]);
247248

248-
useEffect(() => {
249-
if (
250-
focusNotesSearchRequestId === 0 ||
251-
lastHandledFocusNotesSearchRequestIdRef.current ===
252-
focusNotesSearchRequestId
253-
) {
254-
return;
255-
}
256-
257-
lastHandledFocusNotesSearchRequestIdRef.current = focusNotesSearchRequestId;
249+
useCommandRequestId(focusNotesSearchRequestId, () => {
258250
setFocusedPane("notes");
259251
setIsSearchOpen(true);
260252
setIsSearchFocused(true);
261253
focusSearchInput();
262-
}, [focusNotesSearchRequestId, focusSearchInput, setFocusedPane]);
263-
264-
useEffect(() => {
265-
if (
266-
!focusNotesPaneRequest ||
267-
lastHandledFocusNotesPaneRequestIdRef.current ===
268-
focusNotesPaneRequest.requestId
269-
) {
270-
return;
271-
}
254+
});
272255

273-
lastHandledFocusNotesPaneRequestIdRef.current =
274-
focusNotesPaneRequest.requestId;
275-
let selection = focusNotesPaneRequest.selection ?? "selected";
256+
useCommandRequest(focusNotesPaneRequest, (request) => {
257+
let selection = request.selection ?? "selected";
276258

277259
// If the selected note isn't in the current filtered list, fall back
278260
// to selecting the first visible note instead of focusing an empty
@@ -302,14 +284,7 @@ export function NotesPane({
302284
setFocusedPane("notes");
303285
setIsSearchFocused(false);
304286
focusNotesPaneTarget(scrollContainerRef.current);
305-
}, [
306-
focusNotesPaneRequest,
307-
filteredNotes,
308-
isNotesPlaceholderData,
309-
selectFirstVisibleNote,
310-
selectedNoteId,
311-
setFocusedPane,
312-
]);
287+
});
313288

314289
useEffect(() => {
315290
setShowHeaderBorder((scrollContainerRef.current?.scrollTop ?? 0) > 0);
@@ -320,29 +295,21 @@ export function NotesPane({
320295
return;
321296
}
322297

323-
if (pendingNotesPaneSelectionRef.current && isNotesPlaceholderData) {
298+
if (!pendingNotesPaneSelectionRef.current) {
324299
return;
325300
}
326301

327-
// Only respond to filteredNotes.length changes when there is a
328-
// pending selection (initial data load). Without this guard,
329-
// loading more notes via infinite scroll would scroll back to
330-
// the selected note.
331-
if (
332-
!pendingNotesPaneSelectionRef.current &&
333-
!shouldRestoreSelectedRowFocusRef.current
334-
) {
302+
if (isNotesPlaceholderData) {
335303
return;
336304
}
337305

338-
if (selectedNoteId || shouldRestoreSelectedRowFocusRef.current) {
306+
if (selectedNoteId) {
339307
pendingNotesPaneSelectionRef.current = null;
340-
shouldRestoreSelectedRowFocusRef.current = false;
341308
focusSelectedNoteRow(scrollContainerRef.current);
342309
return;
343310
}
344311

345-
if (pendingNotesPaneSelectionRef.current && selectFirstVisibleNote()) {
312+
if (selectFirstVisibleNote()) {
346313
pendingNotesPaneSelectionRef.current = null;
347314
return;
348315
}
@@ -550,9 +517,6 @@ export function NotesPane({
550517
selectedNoteId={selectedNoteId}
551518
setSlideInNoteId={setSlideInNoteId}
552519
shouldSkipAnimation={shouldSkipAnimation}
553-
setShouldRestoreSelectedRowFocus={() => {
554-
shouldRestoreSelectedRowFocusRef.current = true;
555-
}}
556520
/>
557521
);
558522
})}
Lines changed: 25 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { useEffect, useEffectEvent, useRef } from "react";
2-
1+
import { useCommandRequest } from "@/shared/hooks/use-command-request";
32
import { canonicalizeTagPath } from "@/shared/lib/tags";
43
import { useShellCommandStore } from "@/shared/stores/use-shell-command-store";
54
import type { NoteFilter } from "@/shared/api/types";
@@ -45,12 +44,8 @@ export function useShellCommandHandlers(deps: ShellCommandHandlerDeps) {
4544
const focusTagPathRequest = useShellCommandStore(
4645
(state) => state.focusTagPathRequest,
4746
);
48-
const lastHandledCreateNoteFromWikilinkRequestIdRef = useRef(0);
49-
const lastHandledFocusNoteRequestIdRef = useRef(0);
50-
const lastHandledFocusTagPathRequestIdRef = useRef(0);
51-
52-
const handleFocusTagPath = useEffectEvent((requestedTagPath: string) => {
53-
const tagPath = canonicalizeTagPath(requestedTagPath);
47+
useCommandRequest(focusTagPathRequest, (request) => {
48+
const tagPath = canonicalizeTagPath(request.tagPath);
5449
if (!tagPath) {
5550
return;
5651
}
@@ -59,8 +54,8 @@ export function useShellCommandHandlers(deps: ShellCommandHandlerDeps) {
5954
handleSelectTagPath(tagPath);
6055
});
6156

62-
const handleFocusNote = useEffectEvent((requestedNoteId: string) => {
63-
const noteId = requestedNoteId.trim();
57+
useCommandRequest(focusNoteRequest, (request) => {
58+
const noteId = request.noteId.trim();
6459
if (!noteId) {
6560
return;
6661
}
@@ -69,71 +64,30 @@ export function useShellCommandHandlers(deps: ShellCommandHandlerDeps) {
6964
handleSelectNote(noteId);
7065
});
7166

72-
const handleCreateNoteFromWikilink = useEffectEvent(
73-
(request: { location: number; sourceNoteId: string; title: string }) => {
74-
const title = request.title.trim();
75-
if (!title || isCreatingNote) {
76-
return;
77-
}
78-
79-
flushCurrentDraft();
80-
const tagsForNewNote =
81-
tagViewActive && activeTagPath ? [activeTagPath] : [];
82-
if (
83-
!tagViewActive &&
84-
noteFilter !== "today" &&
85-
noteFilter !== "todo" &&
86-
noteFilter !== "pinned" &&
87-
noteFilter !== "untagged"
88-
) {
89-
setNoteFilter("all");
90-
}
91-
setFocusedPane("notes");
92-
prepareNoteCreation();
93-
createNoteMutation.mutate({
94-
autoFocusEditor: false,
95-
tags: tagsForNewNote,
96-
markdown: `# ${title}`,
97-
});
98-
},
99-
);
100-
101-
useEffect(() => {
102-
if (
103-
!focusTagPathRequest ||
104-
lastHandledFocusTagPathRequestIdRef.current ===
105-
focusTagPathRequest.requestId
106-
) {
107-
return;
108-
}
109-
110-
lastHandledFocusTagPathRequestIdRef.current = focusTagPathRequest.requestId;
111-
handleFocusTagPath(focusTagPathRequest.tagPath);
112-
}, [focusTagPathRequest, handleFocusTagPath]);
113-
114-
useEffect(() => {
115-
if (
116-
!focusNoteRequest ||
117-
lastHandledFocusNoteRequestIdRef.current === focusNoteRequest.requestId
118-
) {
67+
useCommandRequest(createNoteFromWikilinkRequest, (request) => {
68+
const title = request.title.trim();
69+
if (!title || isCreatingNote) {
11970
return;
12071
}
12172

122-
lastHandledFocusNoteRequestIdRef.current = focusNoteRequest.requestId;
123-
handleFocusNote(focusNoteRequest.noteId);
124-
}, [focusNoteRequest, handleFocusNote]);
125-
126-
useEffect(() => {
73+
flushCurrentDraft();
74+
const tagsForNewNote =
75+
tagViewActive && activeTagPath ? [activeTagPath] : [];
12776
if (
128-
!createNoteFromWikilinkRequest ||
129-
lastHandledCreateNoteFromWikilinkRequestIdRef.current ===
130-
createNoteFromWikilinkRequest.requestId
77+
!tagViewActive &&
78+
noteFilter !== "today" &&
79+
noteFilter !== "todo" &&
80+
noteFilter !== "pinned" &&
81+
noteFilter !== "untagged"
13182
) {
132-
return;
83+
setNoteFilter("all");
13384
}
134-
135-
lastHandledCreateNoteFromWikilinkRequestIdRef.current =
136-
createNoteFromWikilinkRequest.requestId;
137-
handleCreateNoteFromWikilink(createNoteFromWikilinkRequest);
138-
}, [createNoteFromWikilinkRequest, handleCreateNoteFromWikilink]);
85+
setFocusedPane("notes");
86+
prepareNoteCreation();
87+
createNoteMutation.mutate({
88+
autoFocusEditor: false,
89+
tags: tagsForNewNote,
90+
markdown: `# ${title}`,
91+
});
92+
});
13993
}

0 commit comments

Comments
 (0)