diff --git a/web/src/components/MemoEditor/Editor/index.tsx b/web/src/components/MemoEditor/Editor/index.tsx index 1ce289d0..5a6fa4ba 100644 --- a/web/src/components/MemoEditor/Editor/index.tsx +++ b/web/src/components/MemoEditor/Editor/index.tsx @@ -1,6 +1,6 @@ import { EditorState } from "@codemirror/state"; import { placeholder as cmPlaceholder, EditorView } from "@codemirror/view"; -import { forwardRef, useEffect, useImperativeHandle, useLayoutEffect, useMemo, useRef } from "react"; +import { forwardRef, useCallback, useImperativeHandle, useLayoutEffect, useMemo, useRef } from "react"; import { useTagCounts } from "@/hooks/useUserQueries"; import { cn } from "@/lib/utils"; import type { EditorController } from "../types/editorController"; @@ -12,8 +12,10 @@ import { createFormattingController } from "./formatting"; interface EditorProps { className: string; initialContent: string; + contentIsExternal?: boolean; placeholder: string; onContentChange: (content: string) => void; + onExternalContentApplied?: (content: string) => void; onFiles: (files: File[], position: number) => void; /** Invoked by the in-editor save shortcut (Cmd/Ctrl+Enter). */ onSubmit: () => void; @@ -21,12 +23,26 @@ interface EditorProps { } const Editor = forwardRef(function Editor(props: EditorProps, ref: React.ForwardedRef) { - const { className, initialContent, placeholder, onContentChange, onFiles, onSubmit, isFocusMode } = props; + const { + className, + initialContent, + contentIsExternal = true, + placeholder, + onContentChange, + onExternalContentApplied, + onFiles, + onSubmit, + isFocusMode, + } = props; const hostRef = useRef(null); const viewRef = useRef(null); const controllerRef = useRef(null); + const applyingExternalContentRef = useRef(false); + const pendingExternalContentRef = useRef(null); const onChangeRef = useRef(onContentChange); onChangeRef.current = onContentChange; + const onExternalContentAppliedRef = useRef(onExternalContentApplied); + onExternalContentAppliedRef.current = onExternalContentApplied; const onFilesRef = useRef(onFiles); onFilesRef.current = onFiles; const onSubmitRef = useRef(onSubmit); @@ -40,6 +56,18 @@ const Editor = forwardRef(function Editor(props: EditorProps, ref: React.Forward const tagsRef = useRef(tags); tagsRef.current = tags; + const applyExternalContent = useCallback((content: string) => { + pendingExternalContentRef.current = null; + const controller = controllerRef.current; + if (!controller || controller.getMarkdown() === content) return; + applyingExternalContentRef.current = true; + try { + controller.setMarkdown(content); + } finally { + applyingExternalContentRef.current = false; + } + }, []); + // useLayoutEffect (not useEffect) so the EditorView — and its placeholder — // mount before the browser paints. With useEffect the first painted frame // shows an empty host, then the placeholder pops in (a load flicker). @@ -50,7 +78,9 @@ const Editor = forwardRef(function Editor(props: EditorProps, ref: React.Forward doc: initialContent, extensions: buildEditorExtensions({ placeholder, - onChange: (md) => onChangeRef.current(md), + onChange: (md) => { + if (!applyingExternalContentRef.current) onChangeRef.current(md); + }, onFiles: (files, position) => onFilesRef.current(files, position), onUpdate: () => listenersRef.current.forEach((l) => l()), onSubmit: () => onSubmitRef.current(), @@ -61,7 +91,24 @@ const Editor = forwardRef(function Editor(props: EditorProps, ref: React.Forward }); viewRef.current = view; controllerRef.current = createController(view, createFormattingController(view, listenersRef.current)); + const handleCompositionEnd = () => { + // CodeMirror may flush its final Firefox/Android DOM mutations in a + // microtask after compositionend. Queue behind that flush before + // replacing the document with a deferred external value. + queueMicrotask(() => { + if (viewRef.current !== view || view.compositionStarted) return; + const pendingContent = pendingExternalContentRef.current; + if (pendingContent === null) return; + applyExternalContent(pendingContent); + // The composition may have emitted a newer local value after this + // deferred external value entered the store. Reassert the applied + // external value there too. + onExternalContentAppliedRef.current?.(pendingContent); + }); + }; + view.contentDOM.addEventListener("compositionend", handleCompositionEnd); return () => { + view.contentDOM.removeEventListener("compositionend", handleCompositionEnd); view.destroy(); viewRef.current = null; controllerRef.current = null; @@ -76,12 +123,16 @@ const Editor = forwardRef(function Editor(props: EditorProps, ref: React.Forward viewRef.current?.dispatch({ effects: placeholderCompartment.reconfigure(cmPlaceholder(placeholder)) }); }, [placeholder]); - useEffect(() => { + useLayoutEffect(() => { + if (!contentIsExternal) return; const view = viewRef.current; if (!view) return; - if (view.state.doc.toString() === initialContent) return; - view.dispatch({ changes: { from: 0, to: view.state.doc.length, insert: initialContent } }); - }, [initialContent]); + if (view.compositionStarted) { + pendingExternalContentRef.current = initialContent; + return; + } + applyExternalContent(initialContent); + }, [applyExternalContent, contentIsExternal, initialContent]); // The controller is created in the mount layout effect above, which runs // before this (also layout-phase) handle, so controllerRef.current is set. diff --git a/web/src/components/MemoEditor/components/EditorContent.tsx b/web/src/components/MemoEditor/components/EditorContent.tsx index 5bcdac59..ea4fa05d 100644 --- a/web/src/components/MemoEditor/components/EditorContent.tsx +++ b/web/src/components/MemoEditor/components/EditorContent.tsx @@ -17,21 +17,28 @@ import type { EditorController } from "../types/editorController"; export const EditorContent = forwardRef(({ placeholder, onSubmit, onFiles }, ref) => { const { actions, dispatch } = useEditorContext(); const content = useEditorSelector((s) => s.content); + const contentSource = useEditorSelector((s) => s.contentSource); const isFocusMode = useEditorSelector((s) => s.ui.isFocusMode); const handleContentChange = (content: string) => { dispatch(actions.updateContent(content)); }; + const handleExternalContentApplied = (content: string) => { + dispatch(actions.setContent(content)); + }; + return (
diff --git a/web/src/components/MemoEditor/hooks/useMemoInit.ts b/web/src/components/MemoEditor/hooks/useMemoInit.ts index c06bbe9d..cb29cf7e 100644 --- a/web/src/components/MemoEditor/hooks/useMemoInit.ts +++ b/web/src/components/MemoEditor/hooks/useMemoInit.ts @@ -39,7 +39,7 @@ export const useMemoInit = ({ } else { const cachedDraft = cacheService.loadDraft(key); if (cachedDraft.content) { - dispatch(actions.updateContent(cachedDraft.content)); + dispatch(actions.setContent(cachedDraft.content)); } if (cachedDraft.attachments.length > 0) { dispatch(actions.setMetadata({ attachments: cachedDraft.attachments })); diff --git a/web/src/components/MemoEditor/state/actions.ts b/web/src/components/MemoEditor/state/actions.ts index 5b9ee093..63868ca3 100644 --- a/web/src/components/MemoEditor/state/actions.ts +++ b/web/src/components/MemoEditor/state/actions.ts @@ -9,7 +9,12 @@ export const editorActions = { updateContent: (content: string): EditorAction => ({ type: "UPDATE_CONTENT", - payload: content, + payload: { content, source: "editor" }, + }), + + setContent: (content: string): EditorAction => ({ + type: "UPDATE_CONTENT", + payload: { content, source: "external" }, }), setMetadata: (metadata: Partial): EditorAction => ({ diff --git a/web/src/components/MemoEditor/state/reducer.ts b/web/src/components/MemoEditor/state/reducer.ts index 962a1356..1c9cbfe3 100644 --- a/web/src/components/MemoEditor/state/reducer.ts +++ b/web/src/components/MemoEditor/state/reducer.ts @@ -7,15 +7,21 @@ export function editorReducer(state: EditorState, action: EditorAction): EditorS return { ...state, content: action.payload.content, + contentSource: "external", metadata: action.payload.metadata, timestamps: action.payload.timestamps, }; - case "UPDATE_CONTENT": + case "UPDATE_CONTENT": { + if (state.content === action.payload.content && state.contentSource === action.payload.source) { + return state; + } return { ...state, - content: action.payload, + content: action.payload.content, + contentSource: action.payload.source, }; + } case "SET_METADATA": return { diff --git a/web/src/components/MemoEditor/state/types.ts b/web/src/components/MemoEditor/state/types.ts index e89041b9..8e956694 100644 --- a/web/src/components/MemoEditor/state/types.ts +++ b/web/src/components/MemoEditor/state/types.ts @@ -4,9 +4,11 @@ import { Visibility } from "@/types/proto/api/v1/memo_service_pb"; import type { LocalFile } from "../types/attachment"; export type LoadingKey = "saving" | "uploading" | "loading"; +export type ContentSource = "editor" | "external"; export interface EditorState { content: string; + contentSource: ContentSource; metadata: { visibility: Visibility; attachments: Attachment[]; @@ -34,7 +36,7 @@ export interface EditorState { export type EditorAction = | { type: "INIT_MEMO"; payload: { content: string; metadata: EditorState["metadata"]; timestamps: EditorState["timestamps"] } } - | { type: "UPDATE_CONTENT"; payload: string } + | { type: "UPDATE_CONTENT"; payload: { content: string; source: ContentSource } } | { type: "SET_METADATA"; payload: Partial } | { type: "ADD_LOCAL_FILE"; payload: LocalFile } | { type: "REMOVE_LOCAL_FILE"; payload: string } @@ -49,6 +51,7 @@ export type EditorAction = // Module-private template for createInitialState. const defaultState: EditorState = { content: "", + contentSource: "external", metadata: { visibility: Visibility.PRIVATE, attachments: [], diff --git a/web/tests/editor.test.tsx b/web/tests/editor.test.tsx index 88d347e5..b69d63da 100644 --- a/web/tests/editor.test.tsx +++ b/web/tests/editor.test.tsx @@ -1,4 +1,4 @@ -import { render } from "@testing-library/react"; +import { fireEvent, render, waitFor } from "@testing-library/react"; import { createRef } from "react"; import { describe, expect, it, vi } from "vitest"; import Editor from "@/components/MemoEditor/Editor"; @@ -62,6 +62,58 @@ describe("Editor", () => { expect(onChange).toHaveBeenCalledWith("hello"); }); + it("does not replace newer editor text with a stale local echo", () => { + const ref = createRef(); + const props = { + ref, + className: "x", + placeholder: "memo", + onContentChange: vi.fn(), + onFiles: vi.fn(), + onSubmit: vi.fn(), + }; + const { rerender } = render(); + + ref.current?.setMarkdown("H"); + ref.current?.setMarkdown("Hello"); + rerender(); + + expect(ref.current?.getMarkdown()).toBe("Hello"); + }); + + it("defers external content until an IME composition ends", async () => { + const ref = createRef(); + const onChange = vi.fn(); + const onExternalContentApplied = vi.fn(); + const props = { + ref, + className: "x", + placeholder: "memo", + onContentChange: onChange, + onExternalContentApplied, + onFiles: vi.fn(), + onSubmit: vi.fn(), + contentIsExternal: true, + }; + const { container, rerender } = render(); + const content = container.querySelector(".cm-content"); + expect(content).not.toBeNull(); + + fireEvent.compositionStart(content!); + rerender(); + expect(ref.current?.getMarkdown()).toBe(""); + + // A final IME transaction can arrive after the external value entered the + // store. The deferred external value must still win at compositionend. + ref.current?.setMarkdown("local composition value"); + expect(onChange).toHaveBeenLastCalledWith("local composition value"); + fireEvent.compositionEnd(content!); + + await waitFor(() => expect(ref.current?.getMarkdown()).toBe("server value")); + expect(onExternalContentApplied).toHaveBeenLastCalledWith("server value"); + expect(onChange).not.toHaveBeenCalledWith("server value"); + }); + it("keeps native autocorrection enabled for Windows text services", () => { const props = { className: "x",