fix(web): stabilize rapid image paste selection
This commit is contained in:
1 parent
43194c1b49
commit
cc3feabdf7
5 files changed
+276
-8
No files matched your search
@@ -82,6 +82,12 @@ import { EditorToolbar } from "./EditorToolbar";
|
||||
import { EditorOutline } from "./EditorOutline";
|
||||
import { EditorTagPicker } from "./EditorTagPicker";
|
||||
import { useAiBubbleMenu } from "./editor/useAiBubbleMenu";
|
||||
import {
|
||||
clampResourceInsertionTarget,
|
||||
clearNodeSelectionAtDocumentEnd,
|
||||
getResourceInsertionTarget,
|
||||
shouldSelectInsertedResources,
|
||||
} from "@/lib/resource-insertion-target";
|
||||
import {
|
||||
createSlashCommandExtension,
|
||||
type SlashCommandActions,
|
||||
@@ -190,7 +196,7 @@ import {
|
||||
isAttachmentLinkHref,
|
||||
} from "@/lib/editor-external-link";
|
||||
import { insertAiDraftAtTextCursor } from "@/lib/ai-draft-insertion";
|
||||
import { processFilesSequentially } from "@/lib/file-batch";
|
||||
import { createFileBatchQueue, processFilesSequentially } from "@/lib/file-batch";
|
||||
import { MEMO_ID_REMAPPED_EVENT, MEMO_SYNC_ACKNOWLEDGED_EVENT } from "@/lib/sync-events";
|
||||
import { useStandaloneMobileEditor } from "@/hooks/useStandaloneMobileEditor";
|
||||
import { statusSettleMotion } from "@/lib/motion";
|
||||
@@ -768,6 +774,7 @@ const RichEditorPane = ({
|
||||
const { customEditorTheme, editorTheme } = useEditorTheme();
|
||||
const { markdownTheme } = useMarkdownTheme();
|
||||
const queryClient = useQueryClient();
|
||||
const resourceInsertionLimit = useMemo(createFileBatchQueue, []);
|
||||
const isSelectionMode = Boolean(selectionActionBar);
|
||||
const [title, setTitle] = useState("");
|
||||
const [tagsText, setTagsText] = useState("");
|
||||
@@ -940,6 +947,7 @@ const RichEditorPane = ({
|
||||
const memoRef = useRef<MemoDetail | null>(memo);
|
||||
const editSessionRef = useRef<MemoEditSession | null>(null);
|
||||
const editorRef = useRef<Editor | null>(null);
|
||||
const editorCanvasInteractionVersionRef = useRef(0);
|
||||
const openAiAssistantRef = useRef<() => void>(() => undefined);
|
||||
const aiSpaceShortcutEnabledRef = useRef(readAiSpaceShortcutPreference());
|
||||
const editorScrollContainerRef = useRef<HTMLDivElement | null>(null);
|
||||
@@ -1202,8 +1210,21 @@ const RichEditorPane = ({
|
||||
}
|
||||
|
||||
const targetMemoId = currentMemo.id;
|
||||
const interactionVersionAtRequest = editorCanvasInteractionVersionRef.current;
|
||||
|
||||
void (async () => {
|
||||
void resourceInsertionLimit(async () => {
|
||||
const insertionEditor = editorRef.current;
|
||||
if (
|
||||
memoRef.current?.id !== targetMemoId ||
|
||||
!isEditorReady(insertionEditor) ||
|
||||
!insertionEditor.isEditable
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Read the selection only after earlier resource insertions complete.
|
||||
// Rapid consecutive pastes otherwise race with the same stale cursor.
|
||||
const insertionTarget = getResourceInsertionTarget(insertionEditor.state.selection);
|
||||
setImageUploadState("uploading");
|
||||
|
||||
const results = await processFilesSequentially(files, async (file) => {
|
||||
@@ -1280,7 +1301,27 @@ const RichEditorPane = ({
|
||||
});
|
||||
|
||||
if (content.length > 0) {
|
||||
activeEditor.chain().focus().insertContent(content).run();
|
||||
const safeInsertionTarget = clampResourceInsertionTarget(
|
||||
insertionTarget,
|
||||
activeEditor.state.doc.content.size,
|
||||
);
|
||||
const updateSelection = shouldSelectInsertedResources(
|
||||
interactionVersionAtRequest,
|
||||
editorCanvasInteractionVersionRef.current,
|
||||
);
|
||||
const insertion = activeEditor.chain();
|
||||
if (updateSelection) {
|
||||
insertion.focus();
|
||||
}
|
||||
insertion
|
||||
.insertContentAt(safeInsertionTarget, content, { updateSelection })
|
||||
.run();
|
||||
if (!updateSelection) {
|
||||
// ProseMirror can still map a cursor at the document boundary to a
|
||||
// NodeSelection for the newly inserted block image. Honor the newer
|
||||
// canvas click deterministically instead of trusting that mapping.
|
||||
clearNodeSelectionAtDocumentEnd(activeEditor);
|
||||
}
|
||||
}
|
||||
|
||||
if (results.some((result) => result.status === "rejected")) {
|
||||
@@ -1289,8 +1330,8 @@ const RichEditorPane = ({
|
||||
} else {
|
||||
setImageUploadState("idle");
|
||||
}
|
||||
})();
|
||||
}, [queryClient, repository, t]);
|
||||
});
|
||||
}, [queryClient, repository, resourceInsertionLimit, t]);
|
||||
|
||||
const editor = useEditor({
|
||||
extensions: [
|
||||
@@ -1785,8 +1826,23 @@ const RichEditorPane = ({
|
||||
|
||||
if (event.button === 0 && !event.ctrlKey && !event.metaKey) {
|
||||
showEditorLinkOpenHint(event.target);
|
||||
|
||||
const target = event.target;
|
||||
const proseMirror = event.currentTarget.querySelector<HTMLElement>(".ProseMirror");
|
||||
const clickedEmptyCanvas = target === proseMirror || (
|
||||
target instanceof Node &&
|
||||
proseMirror !== null &&
|
||||
!proseMirror.contains(target)
|
||||
);
|
||||
if (clickedEmptyCanvas) {
|
||||
editorCanvasInteractionVersionRef.current += 1;
|
||||
const clearedNodeSelection = Boolean(editor && clearNodeSelectionAtDocumentEnd(editor));
|
||||
if (editor && clearedNodeSelection) {
|
||||
window.requestAnimationFrame(() => editor.commands.focus());
|
||||
}
|
||||
}
|
||||
}
|
||||
}, [onOpenMemo, showEditorLinkOpenHint]);
|
||||
}, [editor, onOpenMemo, showEditorLinkOpenHint]);
|
||||
|
||||
const handleEditorFocusCapture = useCallback((event: ReactFocusEvent<HTMLDivElement>) => {
|
||||
if (showAttachmentMenu(event.target)) return;
|
||||
@@ -4732,6 +4788,11 @@ const RichEditorPane = ({
|
||||
/>
|
||||
)}
|
||||
<div
|
||||
onClickCapture={
|
||||
!useMobilePlainTextEditor && !useMarkdownSourceEditor
|
||||
? handleEditorClickCapture
|
||||
: undefined
|
||||
}
|
||||
className={cn(
|
||||
"flex gap-8 transition-all duration-200",
|
||||
useMarkdownSourceEditor
|
||||
@@ -4822,7 +4883,6 @@ const RichEditorPane = ({
|
||||
<div
|
||||
onMouseOver={handleEditorMouseOver}
|
||||
onMouseOut={handleEditorMouseOut}
|
||||
onClickCapture={handleEditorClickCapture}
|
||||
onFocusCapture={handleEditorFocusCapture}
|
||||
onBlurCapture={handleEditorBlurCapture}
|
||||
>
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { processFilesSequentially } from "./file-batch.ts";
|
||||
import { createFileBatchQueue, processFilesSequentially } from "./file-batch.ts";
|
||||
|
||||
describe("processFilesSequentially", () => {
|
||||
test("continues after a failed file and preserves selection order", async () => {
|
||||
@@ -26,4 +26,34 @@ describe("processFilesSequentially", () => {
|
||||
]);
|
||||
expect(results[2]).toMatchObject({ status: "fulfilled", value: "last.jpg:uploaded" });
|
||||
});
|
||||
|
||||
test("serializes separate batches triggered by rapid consecutive pastes", async () => {
|
||||
const queue = createFileBatchQueue();
|
||||
const events = [];
|
||||
let finishFirst;
|
||||
let markFirstStarted;
|
||||
const firstCanFinish = new Promise((resolve) => {
|
||||
finishFirst = resolve;
|
||||
});
|
||||
const firstStarted = new Promise((resolve) => {
|
||||
markFirstStarted = resolve;
|
||||
});
|
||||
|
||||
const first = queue(async () => {
|
||||
events.push("first:start");
|
||||
markFirstStarted();
|
||||
await firstCanFinish;
|
||||
events.push("first:end");
|
||||
});
|
||||
const second = queue(async () => {
|
||||
events.push("second:start");
|
||||
events.push("second:end");
|
||||
});
|
||||
|
||||
await firstStarted;
|
||||
expect(events).toEqual(["first:start"]);
|
||||
finishFirst();
|
||||
await Promise.all([first, second]);
|
||||
expect(events).toEqual(["first:start", "first:end", "second:start", "second:end"]);
|
||||
});
|
||||
});
|
||||
@@ -1,3 +1,5 @@
|
||||
import pLimit from "p-limit";
|
||||
|
||||
export type FileBatchResult<T> =
|
||||
| { file: File; status: "fulfilled"; value: T }
|
||||
| { file: File; status: "rejected"; reason: unknown };
|
||||
@@ -18,3 +20,9 @@ export const processFilesSequentially = async <T>(
|
||||
|
||||
return results;
|
||||
};
|
||||
|
||||
/** Serialize separate paste/drop batches so each reads the post-insert cursor. */
|
||||
export const createFileBatchQueue = () => {
|
||||
const limit = pLimit(1);
|
||||
return <T>(task: () => Promise<T>) => limit(task);
|
||||
};
|
||||
@@ -0,0 +1,105 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { Editor } from "@tiptap/core";
|
||||
import Image from "@tiptap/extension-image";
|
||||
import StarterKit from "@tiptap/starter-kit";
|
||||
import {
|
||||
clampResourceInsertionTarget,
|
||||
clearNodeSelectionAtDocumentEnd,
|
||||
getResourceInsertionTarget,
|
||||
shouldSelectInsertedResources,
|
||||
} from "./resource-insertion-target.ts";
|
||||
|
||||
describe("resource insertion target", () => {
|
||||
test("inserts a second pasted image after the selected first image", () => {
|
||||
const editor = new Editor({
|
||||
extensions: [StarterKit, Image.configure({ inline: false })],
|
||||
content: {
|
||||
type: "doc",
|
||||
content: [{ type: "image", attrs: { src: "first.webp" } }],
|
||||
},
|
||||
});
|
||||
editor.commands.setNodeSelection(0);
|
||||
|
||||
const target = getResourceInsertionTarget(editor.state.selection);
|
||||
editor.commands.insertContentAt(target, {
|
||||
type: "image",
|
||||
attrs: { src: "second.webp" },
|
||||
}, { updateSelection: true });
|
||||
|
||||
expect(editor.getJSON().content?.map((node) => node.attrs?.src)).toEqual([
|
||||
"first.webp",
|
||||
"second.webp",
|
||||
]);
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
test("preserves a text selection as the replacement target", () => {
|
||||
const editor = new Editor({
|
||||
extensions: [StarterKit],
|
||||
content: {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", content: [{ type: "text", text: "before after" }] }],
|
||||
},
|
||||
});
|
||||
editor.commands.setTextSelection({ from: 1, to: 7 });
|
||||
|
||||
expect(getResourceInsertionTarget(editor.state.selection)).toEqual({ from: 1, to: 7 });
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
test("clamps a saved target when the document became shorter during upload", () => {
|
||||
expect(clampResourceInsertionTarget({ from: 20, to: 30 }, 8)).toEqual({ from: 8, to: 8 });
|
||||
expect(clampResourceInsertionTarget(20, 8)).toBe(8);
|
||||
});
|
||||
|
||||
test("clears a selected image when the empty editor canvas is clicked", () => {
|
||||
const editor = new Editor({
|
||||
extensions: [StarterKit, Image.configure({ inline: false })],
|
||||
content: {
|
||||
type: "doc",
|
||||
content: [{ type: "image", attrs: { src: "selected.webp" } }],
|
||||
},
|
||||
});
|
||||
editor.commands.setNodeSelection(0);
|
||||
|
||||
expect(clearNodeSelectionAtDocumentEnd(editor)).toBe(true);
|
||||
expect(editor.state.selection.constructor.name).toBe("GapCursor");
|
||||
expect(editor.getJSON().content?.map((node) => node.attrs?.src)).toEqual(["selected.webp"]);
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
test("does not select an asynchronously inserted image after a newer canvas click", () => {
|
||||
expect(shouldSelectInsertedResources(4, 4)).toBe(true);
|
||||
expect(shouldSelectInsertedResources(4, 5)).toBe(false);
|
||||
});
|
||||
|
||||
test("corrects a block image node selection produced by async insertion mapping", () => {
|
||||
const editor = new Editor({
|
||||
extensions: [StarterKit, Image.configure({ inline: false })],
|
||||
content: {
|
||||
type: "doc",
|
||||
content: [{ type: "image", attrs: { src: "first.webp" } }],
|
||||
},
|
||||
});
|
||||
editor.commands.setNodeSelection(0);
|
||||
clearNodeSelectionAtDocumentEnd(editor);
|
||||
|
||||
editor.commands.insertContentAt(
|
||||
editor.state.doc.content.size,
|
||||
{ type: "image", attrs: { src: "second.webp" } },
|
||||
{ updateSelection: false },
|
||||
);
|
||||
// The browser editor's React NodeView can map the boundary selection back
|
||||
// onto the inserted image even though the core Image extension does not.
|
||||
editor.commands.setNodeSelection(1);
|
||||
expect(editor.state.selection.constructor.name).toBe("NodeSelection");
|
||||
|
||||
expect(clearNodeSelectionAtDocumentEnd(editor)).toBe(true);
|
||||
expect(editor.state.selection.constructor.name).toBe("GapCursor");
|
||||
expect(editor.getJSON().content?.map((node) => node.attrs?.src)).toEqual([
|
||||
"first.webp",
|
||||
"second.webp",
|
||||
]);
|
||||
editor.destroy();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,65 @@
|
||||
import type { Editor } from "@tiptap/core";
|
||||
import { GapCursor } from "@tiptap/pm/gapcursor";
|
||||
import type { Selection } from "@tiptap/pm/state";
|
||||
import { NodeSelection, Selection as ProseMirrorSelection } from "@tiptap/pm/state";
|
||||
|
||||
export type ResourceInsertionTarget = number | { from: number; to: number };
|
||||
|
||||
/**
|
||||
* Capture where a pasted or dropped resource belongs before its async upload.
|
||||
* A selected block node (most commonly the previously inserted image) is not
|
||||
* user-selected text to replace, so the next resource belongs after it.
|
||||
*/
|
||||
export const getResourceInsertionTarget = (selection: Selection): ResourceInsertionTarget => {
|
||||
if (selection instanceof NodeSelection) {
|
||||
return selection.to;
|
||||
}
|
||||
|
||||
return { from: selection.from, to: selection.to };
|
||||
};
|
||||
|
||||
export const clampResourceInsertionTarget = (
|
||||
target: ResourceInsertionTarget,
|
||||
documentSize: number,
|
||||
): ResourceInsertionTarget => {
|
||||
if (typeof target === "number") {
|
||||
return Math.max(0, Math.min(target, documentSize));
|
||||
}
|
||||
|
||||
const from = Math.max(0, Math.min(target.from, documentSize));
|
||||
const to = Math.max(from, Math.min(target.to, documentSize));
|
||||
return { from, to };
|
||||
};
|
||||
|
||||
export const shouldSelectInsertedResources = (
|
||||
interactionVersionAtRequest: number,
|
||||
currentInteractionVersion: number,
|
||||
) => interactionVersionAtRequest === currentInteractionVersion;
|
||||
|
||||
/** Clear a selected image/block when the user clicks the empty editor canvas. */
|
||||
export const clearNodeSelectionAtDocumentEnd = (editor: Editor) => {
|
||||
if (!(editor.state.selection instanceof NodeSelection)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const documentEnd = editor.state.doc.content.size;
|
||||
const resolvedEnd = editor.state.doc.resolve(documentEnd);
|
||||
// prosemirror-gapcursor exposes valid() at runtime, but its published
|
||||
// declaration currently omits the static method.
|
||||
const isValidGap = (GapCursor as typeof GapCursor & {
|
||||
valid: (position: typeof resolvedEnd) => boolean;
|
||||
}).valid(resolvedEnd);
|
||||
const nextSelection = isValidGap
|
||||
? new GapCursor(resolvedEnd)
|
||||
: ProseMirrorSelection.atEnd(editor.state.doc);
|
||||
|
||||
// A document containing only a selectable block resolves atEnd() back to
|
||||
// that node. GapCursor handles the normal image-only case; avoid claiming
|
||||
// success for any schema where no non-node selection is available.
|
||||
if (nextSelection instanceof NodeSelection) {
|
||||
return false;
|
||||
}
|
||||
|
||||
editor.view.dispatch(editor.state.tr.setSelection(nextSelection));
|
||||
return true;
|
||||
};
|
||||
Reference in new issue
Block a user