Skip to content

Commit 14d757c

Browse files
committed
fix(editor): preserve IME composition during content sync
1 parent a569775 commit 14d757c

7 files changed

Lines changed: 137 additions & 13 deletions

File tree

web/src/components/MemoEditor/Editor/index.tsx

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { EditorState } from "@codemirror/state";
22
import { placeholder as cmPlaceholder, EditorView } from "@codemirror/view";
3-
import { forwardRef, useEffect, useImperativeHandle, useLayoutEffect, useMemo, useRef } from "react";
3+
import { forwardRef, useCallback, useImperativeHandle, useLayoutEffect, useMemo, useRef } from "react";
44
import { useTagCounts } from "@/hooks/useUserQueries";
55
import { cn } from "@/lib/utils";
66
import type { EditorController } from "../types/editorController";
@@ -12,21 +12,37 @@ import { createFormattingController } from "./formatting";
1212
interface EditorProps {
1313
className: string;
1414
initialContent: string;
15+
contentIsExternal?: boolean;
1516
placeholder: string;
1617
onContentChange: (content: string) => void;
18+
onExternalContentApplied?: (content: string) => void;
1719
onFiles: (files: File[], position: number) => void;
1820
/** Invoked by the in-editor save shortcut (Cmd/Ctrl+Enter). */
1921
onSubmit: () => void;
2022
isFocusMode?: boolean;
2123
}
2224

2325
const Editor = forwardRef(function Editor(props: EditorProps, ref: React.ForwardedRef<EditorController>) {
24-
const { className, initialContent, placeholder, onContentChange, onFiles, onSubmit, isFocusMode } = props;
26+
const {
27+
className,
28+
initialContent,
29+
contentIsExternal = true,
30+
placeholder,
31+
onContentChange,
32+
onExternalContentApplied,
33+
onFiles,
34+
onSubmit,
35+
isFocusMode,
36+
} = props;
2537
const hostRef = useRef<HTMLDivElement>(null);
2638
const viewRef = useRef<EditorView | null>(null);
2739
const controllerRef = useRef<EditorController | null>(null);
40+
const applyingExternalContentRef = useRef(false);
41+
const pendingExternalContentRef = useRef<string | null>(null);
2842
const onChangeRef = useRef(onContentChange);
2943
onChangeRef.current = onContentChange;
44+
const onExternalContentAppliedRef = useRef(onExternalContentApplied);
45+
onExternalContentAppliedRef.current = onExternalContentApplied;
3046
const onFilesRef = useRef(onFiles);
3147
onFilesRef.current = onFiles;
3248
const onSubmitRef = useRef(onSubmit);
@@ -40,6 +56,18 @@ const Editor = forwardRef(function Editor(props: EditorProps, ref: React.Forward
4056
const tagsRef = useRef(tags);
4157
tagsRef.current = tags;
4258

59+
const applyExternalContent = useCallback((content: string) => {
60+
pendingExternalContentRef.current = null;
61+
const controller = controllerRef.current;
62+
if (!controller || controller.getMarkdown() === content) return;
63+
applyingExternalContentRef.current = true;
64+
try {
65+
controller.setMarkdown(content);
66+
} finally {
67+
applyingExternalContentRef.current = false;
68+
}
69+
}, []);
70+
4371
// useLayoutEffect (not useEffect) so the EditorView — and its placeholder —
4472
// mount before the browser paints. With useEffect the first painted frame
4573
// 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
5078
doc: initialContent,
5179
extensions: buildEditorExtensions({
5280
placeholder,
53-
onChange: (md) => onChangeRef.current(md),
81+
onChange: (md) => {
82+
if (!applyingExternalContentRef.current) onChangeRef.current(md);
83+
},
5484
onFiles: (files, position) => onFilesRef.current(files, position),
5585
onUpdate: () => listenersRef.current.forEach((l) => l()),
5686
onSubmit: () => onSubmitRef.current(),
@@ -61,7 +91,24 @@ const Editor = forwardRef(function Editor(props: EditorProps, ref: React.Forward
6191
});
6292
viewRef.current = view;
6393
controllerRef.current = createController(view, createFormattingController(view, listenersRef.current));
94+
const handleCompositionEnd = () => {
95+
// CodeMirror may flush its final Firefox/Android DOM mutations in a
96+
// microtask after compositionend. Queue behind that flush before
97+
// replacing the document with a deferred external value.
98+
queueMicrotask(() => {
99+
if (viewRef.current !== view || view.compositionStarted) return;
100+
const pendingContent = pendingExternalContentRef.current;
101+
if (pendingContent === null) return;
102+
applyExternalContent(pendingContent);
103+
// The composition may have emitted a newer local value after this
104+
// deferred external value entered the store. Reassert the applied
105+
// external value there too.
106+
onExternalContentAppliedRef.current?.(pendingContent);
107+
});
108+
};
109+
view.contentDOM.addEventListener("compositionend", handleCompositionEnd);
64110
return () => {
111+
view.contentDOM.removeEventListener("compositionend", handleCompositionEnd);
65112
view.destroy();
66113
viewRef.current = null;
67114
controllerRef.current = null;
@@ -76,12 +123,16 @@ const Editor = forwardRef(function Editor(props: EditorProps, ref: React.Forward
76123
viewRef.current?.dispatch({ effects: placeholderCompartment.reconfigure(cmPlaceholder(placeholder)) });
77124
}, [placeholder]);
78125

79-
useEffect(() => {
126+
useLayoutEffect(() => {
127+
if (!contentIsExternal) return;
80128
const view = viewRef.current;
81129
if (!view) return;
82-
if (view.state.doc.toString() === initialContent) return;
83-
view.dispatch({ changes: { from: 0, to: view.state.doc.length, insert: initialContent } });
84-
}, [initialContent]);
130+
if (view.compositionStarted) {
131+
pendingExternalContentRef.current = initialContent;
132+
return;
133+
}
134+
applyExternalContent(initialContent);
135+
}, [applyExternalContent, contentIsExternal, initialContent]);
85136

86137
// The controller is created in the mount layout effect above, which runs
87138
// before this (also layout-phase) handle, so controllerRef.current is set.

web/src/components/MemoEditor/components/EditorContent.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,28 @@ import type { EditorController } from "../types/editorController";
1717
export const EditorContent = forwardRef<EditorController, EditorContentProps>(({ placeholder, onSubmit, onFiles }, ref) => {
1818
const { actions, dispatch } = useEditorContext();
1919
const content = useEditorSelector((s) => s.content);
20+
const contentSource = useEditorSelector((s) => s.contentSource);
2021
const isFocusMode = useEditorSelector((s) => s.ui.isFocusMode);
2122

2223
const handleContentChange = (content: string) => {
2324
dispatch(actions.updateContent(content));
2425
};
2526

27+
const handleExternalContentApplied = (content: string) => {
28+
dispatch(actions.setContent(content));
29+
};
30+
2631
return (
2732
<div className="w-full flex flex-col flex-1">
2833
<Editor
2934
ref={ref}
3035
className="memo-editor-content"
3136
initialContent={content}
37+
contentIsExternal={contentSource === "external"}
3238
placeholder={placeholder || ""}
3339
isFocusMode={isFocusMode}
3440
onContentChange={handleContentChange}
41+
onExternalContentApplied={handleExternalContentApplied}
3542
onFiles={onFiles}
3643
onSubmit={onSubmit}
3744
/>

web/src/components/MemoEditor/hooks/useMemoInit.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export const useMemoInit = ({
3939
} else {
4040
const cachedDraft = cacheService.loadDraft(key);
4141
if (cachedDraft.content) {
42-
dispatch(actions.updateContent(cachedDraft.content));
42+
dispatch(actions.setContent(cachedDraft.content));
4343
}
4444
if (cachedDraft.attachments.length > 0) {
4545
dispatch(actions.setMetadata({ attachments: cachedDraft.attachments }));

web/src/components/MemoEditor/state/actions.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@ export const editorActions = {
99

1010
updateContent: (content: string): EditorAction => ({
1111
type: "UPDATE_CONTENT",
12-
payload: content,
12+
payload: { content, source: "editor" },
13+
}),
14+
15+
setContent: (content: string): EditorAction => ({
16+
type: "UPDATE_CONTENT",
17+
payload: { content, source: "external" },
1318
}),
1419

1520
setMetadata: (metadata: Partial<EditorState["metadata"]>): EditorAction => ({

web/src/components/MemoEditor/state/reducer.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,21 @@ export function editorReducer(state: EditorState, action: EditorAction): EditorS
77
return {
88
...state,
99
content: action.payload.content,
10+
contentSource: "external",
1011
metadata: action.payload.metadata,
1112
timestamps: action.payload.timestamps,
1213
};
1314

14-
case "UPDATE_CONTENT":
15+
case "UPDATE_CONTENT": {
16+
if (state.content === action.payload.content && state.contentSource === action.payload.source) {
17+
return state;
18+
}
1519
return {
1620
...state,
17-
content: action.payload,
21+
content: action.payload.content,
22+
contentSource: action.payload.source,
1823
};
24+
}
1925

2026
case "SET_METADATA":
2127
return {

web/src/components/MemoEditor/state/types.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import { Visibility } from "@/types/proto/api/v1/memo_service_pb";
44
import type { LocalFile } from "../types/attachment";
55

66
export type LoadingKey = "saving" | "uploading" | "loading";
7+
export type ContentSource = "editor" | "external";
78

89
export interface EditorState {
910
content: string;
11+
contentSource: ContentSource;
1012
metadata: {
1113
visibility: Visibility;
1214
attachments: Attachment[];
@@ -34,7 +36,7 @@ export interface EditorState {
3436

3537
export type EditorAction =
3638
| { type: "INIT_MEMO"; payload: { content: string; metadata: EditorState["metadata"]; timestamps: EditorState["timestamps"] } }
37-
| { type: "UPDATE_CONTENT"; payload: string }
39+
| { type: "UPDATE_CONTENT"; payload: { content: string; source: ContentSource } }
3840
| { type: "SET_METADATA"; payload: Partial<EditorState["metadata"]> }
3941
| { type: "ADD_LOCAL_FILE"; payload: LocalFile }
4042
| { type: "REMOVE_LOCAL_FILE"; payload: string }
@@ -49,6 +51,7 @@ export type EditorAction =
4951
// Module-private template for createInitialState.
5052
const defaultState: EditorState = {
5153
content: "",
54+
contentSource: "external",
5255
metadata: {
5356
visibility: Visibility.PRIVATE,
5457
attachments: [],

web/tests/editor.test.tsx

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { render } from "@testing-library/react";
1+
import { fireEvent, render, waitFor } from "@testing-library/react";
22
import { createRef } from "react";
33
import { describe, expect, it, vi } from "vitest";
44
import Editor from "@/components/MemoEditor/Editor";
@@ -62,6 +62,58 @@ describe("Editor", () => {
6262
expect(onChange).toHaveBeenCalledWith("hello");
6363
});
6464

65+
it("does not replace newer editor text with a stale local echo", () => {
66+
const ref = createRef<EditorController>();
67+
const props = {
68+
ref,
69+
className: "x",
70+
placeholder: "memo",
71+
onContentChange: vi.fn(),
72+
onFiles: vi.fn(),
73+
onSubmit: vi.fn(),
74+
};
75+
const { rerender } = render(<Editor {...props} initialContent="" />);
76+
77+
ref.current?.setMarkdown("H");
78+
ref.current?.setMarkdown("Hello");
79+
rerender(<Editor {...props} initialContent="H" contentIsExternal={false} />);
80+
81+
expect(ref.current?.getMarkdown()).toBe("Hello");
82+
});
83+
84+
it("defers external content until an IME composition ends", async () => {
85+
const ref = createRef<EditorController>();
86+
const onChange = vi.fn();
87+
const onExternalContentApplied = vi.fn();
88+
const props = {
89+
ref,
90+
className: "x",
91+
placeholder: "memo",
92+
onContentChange: onChange,
93+
onExternalContentApplied,
94+
onFiles: vi.fn(),
95+
onSubmit: vi.fn(),
96+
contentIsExternal: true,
97+
};
98+
const { container, rerender } = render(<Editor {...props} initialContent="" />);
99+
const content = container.querySelector<HTMLElement>(".cm-content");
100+
expect(content).not.toBeNull();
101+
102+
fireEvent.compositionStart(content!);
103+
rerender(<Editor {...props} initialContent="server value" />);
104+
expect(ref.current?.getMarkdown()).toBe("");
105+
106+
// A final IME transaction can arrive after the external value entered the
107+
// store. The deferred external value must still win at compositionend.
108+
ref.current?.setMarkdown("local composition value");
109+
expect(onChange).toHaveBeenLastCalledWith("local composition value");
110+
fireEvent.compositionEnd(content!);
111+
112+
await waitFor(() => expect(ref.current?.getMarkdown()).toBe("server value"));
113+
expect(onExternalContentApplied).toHaveBeenLastCalledWith("server value");
114+
expect(onChange).not.toHaveBeenCalledWith("server value");
115+
});
116+
65117
it("keeps native autocorrection enabled for Windows text services", () => {
66118
const props = {
67119
className: "x",

0 commit comments

Comments
 (0)