fix(desktop): keep one new note from appearing twice
Cmd+N and a second click could both create before isPending flipped. Sync also left the local id in the list beside the durable id, and autofocus stopped before the editor was editable.
This commit is contained in:
1 parent
4f753587f6
commit
df7931e2f6
10 files changed
+245
-23
No files matched your search
@@ -253,8 +253,10 @@ import {
|
||||
getNoteLinkFromEventTarget,
|
||||
getNoteLinkHintPosition,
|
||||
getResourceFilesFromDataTransfer,
|
||||
isCreatedMemoEditorFocused,
|
||||
isEditorReady,
|
||||
MemoSaveRequestError,
|
||||
shouldRetryCreatedMemoFocus,
|
||||
MOBILE_DRAFT_PERSIST_DELAY_MS,
|
||||
MOBILE_EDITOR_QUERY,
|
||||
requiresLocalEditSession,
|
||||
@@ -782,27 +784,51 @@ const RichEditorPane = ({
|
||||
}
|
||||
|
||||
const currentEditor = editorRef.current;
|
||||
if (!isMobileViewport) {
|
||||
if (isEditorReady(currentEditor) && hydratedMemoIdRef.current === memo.id) {
|
||||
currentEditor.commands.focus("end");
|
||||
// Consuming the create request updates the parent and can
|
||||
// briefly blur the editor during that rerender. Mobile
|
||||
// standalone editing consumes the request above; desktop
|
||||
// keeps it alive until the list has observed the new memo so
|
||||
// a background refresh cannot select the previous memo.
|
||||
window.setTimeout(() => {
|
||||
if (cancelled || memoRef.current?.id !== memo.id) {
|
||||
return;
|
||||
}
|
||||
if (!isMobileViewport) {
|
||||
const hydratedForMemo = isEditorInstanceHydratedForMemo(
|
||||
editorInstanceMemoIdentityRef.current,
|
||||
hydratedMemoIdRef.current,
|
||||
memo.id,
|
||||
);
|
||||
if (isEditorReady(currentEditor) && hydratedForMemo && currentEditor.isEditable) {
|
||||
currentEditor.commands.focus("end");
|
||||
}
|
||||
|
||||
const activeEditor = editorRef.current;
|
||||
if (isEditorReady(activeEditor)) {
|
||||
activeEditor.commands.focus("end");
|
||||
}
|
||||
}, 0);
|
||||
if (shouldRetryCreatedMemoFocus({
|
||||
attempt,
|
||||
editorEditable: Boolean(currentEditor?.isEditable),
|
||||
editorFocused: isCreatedMemoEditorFocused(currentEditor),
|
||||
editorReady: isEditorReady(currentEditor),
|
||||
hydratedForMemo,
|
||||
})) {
|
||||
focusWhenReady(attempt + 1);
|
||||
return;
|
||||
}
|
||||
|
||||
// Consuming the create request updates the parent and can
|
||||
// briefly blur the editor during that rerender. Desktop sync
|
||||
// remapping can also change the memo id before this timeout.
|
||||
window.setTimeout(() => {
|
||||
if (cancelled) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const activeMemoId = memoRef.current?.id;
|
||||
if (
|
||||
!activeMemoId
|
||||
|| !editorInstanceMemoIdentityRef.current.aliases.has(activeMemoId)
|
||||
|| !editorInstanceMemoIdentityRef.current.aliases.has(memo.id)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
const activeEditor = editorRef.current;
|
||||
if (isEditorReady(activeEditor) && activeEditor.isEditable) {
|
||||
activeEditor.commands.focus("end");
|
||||
}
|
||||
}, 0);
|
||||
return;
|
||||
}
|
||||
|
||||
// The editor is mounted before its memo hydration/edit session
|
||||
// finishes. Keep retrying across that async boundary so a newly
|
||||
@@ -1667,6 +1693,7 @@ const RichEditorPane = ({
|
||||
contentSearchQuery,
|
||||
dirtyVersion,
|
||||
editor,
|
||||
editorInstanceKey: editorInstanceMemoKey,
|
||||
editorScrollContainerRef,
|
||||
memoId: memo?.id ?? null,
|
||||
readOnly: effectiveReadOnly,
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { readFileSync } from "node:fs";
|
||||
|
||||
const workspaceSource = readFileSync(new URL("./WorkspaceApp.tsx", import.meta.url), "utf8");
|
||||
const editorSource = readFileSync(new URL("./EditorPane.tsx", import.meta.url), "utf8");
|
||||
const searchControllerSource = readFileSync(new URL("./editor/useEditorNoteSearchController.ts", import.meta.url), "utf8");
|
||||
const queuedSyncSource = readFileSync(new URL("../hooks/useWorkspaceQueuedSync.ts", import.meta.url), "utf8");
|
||||
|
||||
describe("desktop create-note wiring", () => {
|
||||
test("ignores a second new-note request while the first create is still in flight", () => {
|
||||
expect(workspaceSource).toContain("createMemoInFlightRef");
|
||||
expect(workspaceSource).toContain("createMemoInFlightRef.current || createMemoMutation.isPending");
|
||||
expect(workspaceSource).toContain("createMemoInFlightRef.current = false");
|
||||
});
|
||||
|
||||
test("remaps the memo list when a local create id becomes the durable id", () => {
|
||||
expect(queuedSyncSource).toContain("remapMemoIdsInLists(queryClient, memoIdMappings)");
|
||||
expect(queuedSyncSource).toContain("MEMO_ID_REMAPPED_EVENT");
|
||||
});
|
||||
|
||||
test("resets in-note search on editor instance changes, not local-to-remote id remaps", () => {
|
||||
expect(searchControllerSource).toContain("editorInstanceKey");
|
||||
expect(searchControllerSource).toContain("shouldResetNoteSearchForMemoChange(previousEditorInstanceKeyRef.current, editorInstanceKey)");
|
||||
expect(editorSource).toContain("editorInstanceKey: editorInstanceMemoKey");
|
||||
});
|
||||
|
||||
test("keeps retrying create-note autofocus until the editor is editable and focused", () => {
|
||||
expect(editorSource).toContain("shouldRetryCreatedMemoFocus");
|
||||
expect(editorSource).toContain("isCreatedMemoEditorFocused");
|
||||
expect(editorSource).toContain("currentEditor.isEditable");
|
||||
});
|
||||
});
|
||||
@@ -229,6 +229,7 @@ export const WorkspaceApp = ({
|
||||
const pendingCreatedMemoIdRef = useRef<string | null>(null);
|
||||
const pendingQuickSwitcherMemoIdRef = useRef<string | null>(null); // also companion/plugin opens not yet in the list
|
||||
const creatingMemoSelectionRef = useRef(false);
|
||||
const createMemoInFlightRef = useRef(false);
|
||||
const memoDocumentActionIdRef = useRef(0);
|
||||
const [memoDocumentActionRequest, setMemoDocumentActionRequest] = useState<MemoDocumentActionRequest | null>(null);
|
||||
const [memoDeleteConfirmation, setMemoDeleteConfirmation] = useState<MemoDeleteConfirmation | null>(null);
|
||||
@@ -1236,6 +1237,9 @@ export const WorkspaceApp = ({
|
||||
clearPendingCreatedMemo();
|
||||
setCreatedMemoEditId(null);
|
||||
},
|
||||
onSettled: () => {
|
||||
createMemoInFlightRef.current = false;
|
||||
},
|
||||
});
|
||||
|
||||
const saveTemplateMutation = useMutation({
|
||||
@@ -1718,6 +1722,13 @@ export const WorkspaceApp = ({
|
||||
return;
|
||||
}
|
||||
|
||||
// Desktop Cmd+N is handled by both the native menu and the in-app shortcut.
|
||||
// A second click can also land before React Query flips `isPending`.
|
||||
if (createMemoInFlightRef.current || createMemoMutation.isPending) {
|
||||
return;
|
||||
}
|
||||
createMemoInFlightRef.current = true;
|
||||
|
||||
setTemplatesOpen(false);
|
||||
setMobileBottomNavActive("home");
|
||||
creatingMemoSelectionRef.current = true;
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { shouldRetryCreatedMemoFocus } from "./editor-pane-helpers.ts";
|
||||
|
||||
describe("created memo focus retry", () => {
|
||||
test("keeps retrying until the editor is hydrated, editable, and focused", () => {
|
||||
expect(shouldRetryCreatedMemoFocus({
|
||||
attempt: 0,
|
||||
editorReady: false,
|
||||
editorEditable: false,
|
||||
editorFocused: false,
|
||||
hydratedForMemo: false,
|
||||
})).toBe(true);
|
||||
expect(shouldRetryCreatedMemoFocus({
|
||||
attempt: 3,
|
||||
editorReady: true,
|
||||
editorEditable: false,
|
||||
editorFocused: false,
|
||||
hydratedForMemo: true,
|
||||
})).toBe(true);
|
||||
expect(shouldRetryCreatedMemoFocus({
|
||||
attempt: 4,
|
||||
editorReady: true,
|
||||
editorEditable: true,
|
||||
editorFocused: false,
|
||||
hydratedForMemo: true,
|
||||
})).toBe(true);
|
||||
expect(shouldRetryCreatedMemoFocus({
|
||||
attempt: 5,
|
||||
editorReady: true,
|
||||
editorEditable: true,
|
||||
editorFocused: true,
|
||||
hydratedForMemo: true,
|
||||
})).toBe(false);
|
||||
});
|
||||
|
||||
test("stops retrying after the attempt budget", () => {
|
||||
expect(shouldRetryCreatedMemoFocus({
|
||||
attempt: 120,
|
||||
editorReady: true,
|
||||
editorEditable: false,
|
||||
editorFocused: false,
|
||||
hydratedForMemo: true,
|
||||
})).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -77,6 +77,36 @@ export type MobilePlainTextElement = HTMLTextAreaElement | HTMLDivElement;
|
||||
export const isEditorReady = (editor: Editor | null | undefined): editor is Editor =>
|
||||
Boolean(editor && !editor.isDestroyed && (editor as { extensionManager?: unknown }).extensionManager);
|
||||
|
||||
export const CREATED_MEMO_FOCUS_MAX_ATTEMPTS = 120;
|
||||
|
||||
export const isCreatedMemoEditorFocused = (editor: Editor | null | undefined) =>
|
||||
isEditorReady(editor) && editor.isEditable && (editor.isFocused || editor.view.hasFocus());
|
||||
|
||||
/**
|
||||
* Keep retrying create-note autofocus until the editor is actually editable
|
||||
* and focused. Hydration sets a ref before React has flipped `editable`, and
|
||||
* desktop id remapping can blur a successful first focus.
|
||||
*/
|
||||
export const shouldRetryCreatedMemoFocus = ({
|
||||
attempt,
|
||||
editorEditable,
|
||||
editorFocused,
|
||||
editorReady,
|
||||
hydratedForMemo,
|
||||
maxAttempts = CREATED_MEMO_FOCUS_MAX_ATTEMPTS,
|
||||
}: {
|
||||
attempt: number;
|
||||
editorEditable: boolean;
|
||||
editorFocused: boolean;
|
||||
editorReady: boolean;
|
||||
hydratedForMemo: boolean;
|
||||
maxAttempts?: number;
|
||||
}) => {
|
||||
if (attempt >= maxAttempts) return false;
|
||||
if (!editorReady || !hydratedForMemo || !editorEditable) return true;
|
||||
return !editorFocused;
|
||||
};
|
||||
|
||||
export const getMobilePlainTextElementValue = (element: MobilePlainTextElement | null) => {
|
||||
if (!element) {
|
||||
return "";
|
||||
|
||||
@@ -63,6 +63,8 @@ describe("note search", () => {
|
||||
expect(shouldResetNoteSearchForMemoChange("note-a", "note-a")).toBe(false);
|
||||
expect(shouldResetNoteSearchForMemoChange(null, "note-b")).toBe(true);
|
||||
expect(shouldResetNoteSearchForMemoChange(null, null)).toBe(false);
|
||||
// Desktop create keeps the editor instance key while the local id remaps.
|
||||
expect(shouldResetNoteSearchForMemoChange("memo_local_1", "memo_local_1")).toBe(false);
|
||||
expect(shouldDiscardPluginNoteSearchRequest(
|
||||
{ noteId: "note-a" },
|
||||
"note-b",
|
||||
|
||||
@@ -23,6 +23,7 @@ type EditorNoteSearchControllerOptions = {
|
||||
contentSearchQuery: string;
|
||||
dirtyVersion: number;
|
||||
editor: Editor | null;
|
||||
editorInstanceKey: string | null;
|
||||
editorScrollContainerRef: RefObject<HTMLDivElement | null>;
|
||||
readOnly: boolean;
|
||||
replaceFocusToken: number;
|
||||
@@ -45,6 +46,7 @@ export const useEditorNoteSearchController = ({
|
||||
contentSearchQuery,
|
||||
dirtyVersion,
|
||||
editor,
|
||||
editorInstanceKey,
|
||||
editorScrollContainerRef,
|
||||
memoId,
|
||||
readOnly,
|
||||
@@ -58,9 +60,11 @@ export const useEditorNoteSearchController = ({
|
||||
const [noteSearchIndex, setNoteSearchIndex] = useState(0);
|
||||
const noteSearchInputRef = useRef<HTMLInputElement | null>(null);
|
||||
const automaticSelectionRef = useRef<{ editor: Editor; identity: string } | null>(null);
|
||||
const previousMemoIdRef = useRef(memoId);
|
||||
if (shouldResetNoteSearchForMemoChange(previousMemoIdRef.current, memoId)) {
|
||||
previousMemoIdRef.current = memoId;
|
||||
const previousEditorInstanceKeyRef = useRef(editorInstanceKey);
|
||||
// Reset on a real note switch, not when desktop sync remaps a local id.
|
||||
// The editor instance key stays stable across that handoff.
|
||||
if (shouldResetNoteSearchForMemoChange(previousEditorInstanceKeyRef.current, editorInstanceKey)) {
|
||||
previousEditorInstanceKeyRef.current = editorInstanceKey;
|
||||
setNoteSearchOpen(CLOSED_NOTE_SEARCH_STATE.open);
|
||||
setNoteSearchQuery(CLOSED_NOTE_SEARCH_STATE.query);
|
||||
setNoteSearchReplaceOpen(CLOSED_NOTE_SEARCH_STATE.replaceOpen);
|
||||
|
||||
@@ -3,8 +3,9 @@ import type { QueryClient } from "@tanstack/react-query";
|
||||
import type { MemoDetail, MemoTemplate, Notebook, Resource } from "@edgeever/shared";
|
||||
import { isBrowserOffline } from "@/lib/network-status";
|
||||
import { emptySyncQueueSummary, type SyncQueueSummary } from "@/lib/sync-queue";
|
||||
import { notifyMemoIdRemapped, notifyMemoSyncAcknowledged } from "@/lib/sync-events";
|
||||
import { MEMO_ID_REMAPPED_EVENT, notifyMemoIdRemapped, notifyMemoSyncAcknowledged } from "@/lib/sync-events";
|
||||
import { observeLocalMemoIdMappings, putLocalMemo, replaceLocalMemoId } from "@/lib/local-mirror";
|
||||
import { remapMemoIdsInLists } from "@/lib/memo-list-cache";
|
||||
import { preserveRemappedMemoDetailQueries, resolveSyncedMemoId } from "@/lib/workspace-refresh";
|
||||
|
||||
type UseWorkspaceQueuedSyncOptions = {
|
||||
@@ -38,6 +39,10 @@ export const useWorkspaceQueuedSync = ({
|
||||
// editor otherwise renders its loading state between the local-id and
|
||||
// remote-id queries, which looks like a full editor reload after create.
|
||||
preserveRemappedMemoDetailQueries(queryClient, memoIdMappings);
|
||||
// The create path inserts the temporary id into the list cache. If sync
|
||||
// later adds the durable id without replacing that row, the list shows two
|
||||
// empty notes for one create.
|
||||
remapMemoIdsInLists(queryClient, memoIdMappings);
|
||||
notifyMemoIdRemapped(memoIdMappings);
|
||||
const selectedMemoId = selectedMemoIdRef.current;
|
||||
const pendingMemoId = pendingCreatedMemoIdRef.current;
|
||||
@@ -59,6 +64,15 @@ export const useWorkspaceQueuedSync = ({
|
||||
|
||||
useEffect(() => observeLocalMemoIdMappings(localDataScope, applyMemoIdMappings), [applyMemoIdMappings, localDataScope]);
|
||||
|
||||
useEffect(() => {
|
||||
const handleMemoIdRemapped = (event: Event) => {
|
||||
const mappings = (event as CustomEvent<ReadonlyMap<string, string>>).detail;
|
||||
if (mappings instanceof Map) remapMemoIdsInLists(queryClient, mappings);
|
||||
};
|
||||
window.addEventListener(MEMO_ID_REMAPPED_EVENT, handleMemoIdRemapped);
|
||||
return () => window.removeEventListener(MEMO_ID_REMAPPED_EVENT, handleMemoIdRemapped);
|
||||
}, [queryClient]);
|
||||
|
||||
const invalidateSyncQueries = useCallback(() => Promise.all([
|
||||
queryClient.invalidateQueries({ queryKey: ["memos"] }),
|
||||
queryClient.invalidateQueries({ queryKey: ["memo"] }),
|
||||
|
||||
@@ -109,6 +109,47 @@ const reflowMemoListPages = (current: MemoListQueryData, memos: MemoSummary[], t
|
||||
};
|
||||
};
|
||||
|
||||
export const remapMemoIdsInLists = (
|
||||
queryClient: QueryClient,
|
||||
memoIdMappings: ReadonlyMap<string, string>,
|
||||
) => {
|
||||
if (memoIdMappings.size === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const [queryKey, current] of queryClient.getQueriesData<MemoListQueryData>({ queryKey: ["memos"] })) {
|
||||
if (!current) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const flatMemos = current.pages.flatMap((page) => page.memos);
|
||||
let changed = false;
|
||||
const remappedMemos = flatMemos.map((memo) => {
|
||||
const nextId = memoIdMappings.get(memo.id);
|
||||
if (!nextId || nextId === memo.id) {
|
||||
return memo;
|
||||
}
|
||||
|
||||
changed = true;
|
||||
return { ...memo, id: nextId };
|
||||
});
|
||||
|
||||
if (!changed) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const nextMemos = deduplicateMemoSummaries(remappedMemos);
|
||||
const removed = remappedMemos.length - nextMemos.length;
|
||||
const currentTotalCount = current.pages[0]?.totalCount ?? flatMemos.length;
|
||||
const totalCount = Math.max(nextMemos.length, currentTotalCount - removed);
|
||||
|
||||
queryClient.setQueryData(
|
||||
queryKey,
|
||||
reflowMemoListPages(current, sortMemoSummariesForList(nextMemos, queryKey), totalCount),
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
export const updateMemoSummaryInLists = (queryClient: QueryClient, summary: MemoSummary) => {
|
||||
for (const [queryKey, current] of queryClient.getQueriesData<MemoListQueryData>({ queryKey: ["memos"] })) {
|
||||
if (!current) {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { QueryClient } from "@tanstack/react-query";
|
||||
import type { MemoSummary } from "@edgeever/shared";
|
||||
import { updateMemoSummaryInLists, type MemoListQueryData } from "../apps/web/src/lib/memo-list-cache";
|
||||
import { remapMemoIdsInLists, updateMemoSummaryInLists, type MemoListQueryData } from "../apps/web/src/lib/memo-list-cache";
|
||||
|
||||
const memo = (overrides: Partial<MemoSummary> = {}): MemoSummary => ({
|
||||
id: "memo-apple",
|
||||
@@ -62,6 +62,22 @@ describe("memo list cache updates", () => {
|
||||
expect(cached?.pages[0]?.totalCount).toBe(1);
|
||||
});
|
||||
|
||||
test("replaces a temporary create id so the list does not show two empty notes", () => {
|
||||
const queryClient = new QueryClient();
|
||||
const queryKey = ["memos", "notebook", "nb-inbox", "", "all", "updated-desc"] as const;
|
||||
queryClient.setQueryData(queryKey, queryData([
|
||||
[memo({ id: "memo_remote_1", title: "", excerpt: "" }), memo({ id: "memo_local_1", title: "", excerpt: "" })],
|
||||
], 2));
|
||||
|
||||
remapMemoIdsInLists(queryClient, new Map([["memo_local_1", "memo_remote_1"]]));
|
||||
|
||||
const cached = queryClient.getQueryData<MemoListQueryData>(queryKey);
|
||||
const flattened = cached?.pages.flatMap((page) => page.memos) ?? [];
|
||||
expect(flattened).toHaveLength(1);
|
||||
expect(flattened[0]?.id).toBe("memo_remote_1");
|
||||
expect(cached?.pages[0]?.totalCount).toBe(1);
|
||||
});
|
||||
|
||||
test("removes a note when an edit no longer matches the selected exact tag", () => {
|
||||
const queryClient = new QueryClient();
|
||||
const queryKey = ["memos", "notebook", null, "", "all", "updated-desc", [], "demo"] as const;
|
||||
|
||||
Reference in new issue
Block a user