diff --git a/packages/views/editor/content-editor.test.tsx b/packages/views/editor/content-editor.test.tsx index 5b3c612d1..f0511f971 100644 --- a/packages/views/editor/content-editor.test.tsx +++ b/packages/views/editor/content-editor.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { act, fireEvent, render, screen } from "@testing-library/react"; import type { Attachment } from "@multica/core/types"; import type { UploadResult } from "@multica/core/hooks/use-file-upload"; @@ -57,9 +57,16 @@ vi.mock("./attachment-download-context", () => ({ const editorRef = vi.hoisted<{ current: unknown }>(() => ({ current: null })); const onCreateFired = vi.hoisted(() => ({ value: false })); +const latestEditorOptions = vi.hoisted<{ + current?: { onUpdate?: (args: { editor: unknown }) => void }; +}>(() => ({})); vi.mock("@tiptap/react", () => ({ - useEditor: (options: { onCreate?: (args: { editor: unknown }) => void }) => { + useEditor: (options: { + onCreate?: (args: { editor: unknown }) => void; + onUpdate?: (args: { editor: unknown }) => void; + }) => { + latestEditorOptions.current = options; if (!editorRef.current) { editorRef.current = { get isFocused() { @@ -104,9 +111,14 @@ describe("ContentEditor", () => { editorState.markdown = ""; editorRef.current = null; onCreateFired.value = false; + latestEditorOptions.current = undefined; providerProps.attachments = undefined; }); + afterEach(() => { + vi.useRealTimers(); + }); + it("focuses the editor when clicking the empty container area", () => { render(); @@ -210,6 +222,98 @@ describe("ContentEditor", () => { expect(mockSetContent).not.toHaveBeenCalled(); }); + + it("flushes a pending debounced update on unmount when flushPendingOnUnmount is set", () => { + vi.useFakeTimers(); + const onUpdate = vi.fn(); + editorState.markdown = "old content"; + const { unmount } = render( + , + ); + + editorState.markdown = "old content\n\n![shot](/api/attachments/att-1/download)"; + act(() => { + latestEditorOptions.current?.onUpdate?.({ editor: editorRef.current }); + }); + + expect(onUpdate).not.toHaveBeenCalled(); + + // The flush must emit the copy cached at onUpdate time — by cleanup time + // Tiptap may already have torn the instance down, so reading the editor + // during unmount is not an option. + editorState.isDestroyed = true; + editorState.markdown = ""; + + unmount(); + + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith( + "old content\n\n![shot](/api/attachments/att-1/download)", + ); + act(() => { + vi.advanceTimersByTime(1500); + }); + expect(onUpdate).toHaveBeenCalledTimes(1); + }); + + it("drops a pending debounced update on unmount by default", () => { + // Regression guard for draft resurrection: composers like comment edit + // cancel `clearDraft()` and then unmount this editor. A default unmount + // flush would re-emit the discarded markdown into onUpdate, which writes + // it straight back into the draft store. + vi.useFakeTimers(); + const onUpdate = vi.fn(); + editorState.markdown = "edit draft the user cancelled"; + const { unmount } = render( + , + ); + + act(() => { + latestEditorOptions.current?.onUpdate?.({ editor: editorRef.current }); + }); + expect(onUpdate).not.toHaveBeenCalled(); + + unmount(); + + expect(onUpdate).not.toHaveBeenCalled(); + act(() => { + vi.advanceTimersByTime(300); + }); + expect(onUpdate).not.toHaveBeenCalled(); + }); + + it("does not re-emit on unmount when the debounce already fired", () => { + vi.useFakeTimers(); + const onUpdate = vi.fn(); + const { unmount } = render( + , + ); + + editorState.markdown = "typed content"; + act(() => { + latestEditorOptions.current?.onUpdate?.({ editor: editorRef.current }); + vi.advanceTimersByTime(1500); + }); + expect(onUpdate).toHaveBeenCalledTimes(1); + + unmount(); + + expect(onUpdate).toHaveBeenCalledTimes(1); + }); }); function makeAttachment(id: string, overrides: Partial = {}): Attachment { diff --git a/packages/views/editor/content-editor.tsx b/packages/views/editor/content-editor.tsx index 14eb3bf44..7e3d6c35c 100644 --- a/packages/views/editor/content-editor.tsx +++ b/packages/views/editor/content-editor.tsx @@ -124,6 +124,17 @@ interface ContentEditorProps { * available (NodeView buttons fall back to opening the raw URL). */ attachments?: Attachment[]; + /** + * Flush a pending debounced `onUpdate` when the editor unmounts instead of + * dropping it. Default false ON PURPOSE: most composers clear their draft + * and then unmount (comment edit cancel, create-issue / feedback submit), + * and a flush there would hand the discarded content right back to + * `onUpdate`, resurrecting the cleared draft. Opt in only where closing + * means "keep what the user last saw" — e.g. the issue-detail description + * editor, whose 1500ms debounce would otherwise drop a paste made just + * before the modal closes. + */ + flushPendingOnUnmount?: boolean; } interface ContentEditorRef { @@ -163,10 +174,16 @@ const ContentEditor = forwardRef( enableSlashCommands = false, slashCommandMode = "skill", attachments, + flushPendingOnUnmount = false, }, ref, ) { const debounceRef = useRef>(undefined); + const flushPendingOnUnmountRef = useRef(flushPendingOnUnmount); + // Markdown serialized at `onUpdate` time, awaiting its debounce fire. The + // unmount flush emits this cached copy — it runs mid-teardown and can't + // assume the editor instance is still readable. + const pendingFlushRef = useRef(null); const onUpdateRef = useRef(onUpdate); const onSubmitRef = useRef(onSubmit); const onBlurRef = useRef(onBlur); @@ -249,6 +266,7 @@ const ContentEditor = forwardRef( onBlurRef.current = onBlur; onUploadFileRef.current = wrappedOnUploadFile; mentionContextItemsRef.current = mentionContextItems ?? []; + flushPendingOnUnmountRef.current = flushPendingOnUnmount; const queryClient = useQueryClient(); @@ -303,8 +321,13 @@ const ContentEditor = forwardRef( }), onUpdate: ({ editor: ed }) => { if (!onUpdateRef.current) return; + if (flushPendingOnUnmountRef.current) { + pendingFlushRef.current = stripBlobUrls(ed.getMarkdown()).trimEnd(); + } if (debounceRef.current) clearTimeout(debounceRef.current); debounceRef.current = setTimeout(() => { + debounceRef.current = undefined; + pendingFlushRef.current = null; const md = stripBlobUrls(ed.getMarkdown()).trimEnd(); if (md === lastEmittedRef.current) return; lastEmittedRef.current = md; @@ -336,10 +359,21 @@ const ContentEditor = forwardRef( }, }); - // Cleanup debounce on unmount + // Cleanup on unmount. A pending debounced update is DROPPED by default, + // not flushed — see the `flushPendingOnUnmount` prop doc for why. When the + // owner opted in, emit the markdown cached at `onUpdate` time so a long + // debounce can't swallow the last edit when the surrounding modal closes. useEffect(() => { return () => { - if (debounceRef.current) clearTimeout(debounceRef.current); + if (!debounceRef.current) return; + clearTimeout(debounceRef.current); + debounceRef.current = undefined; + if (!flushPendingOnUnmountRef.current) return; + const pending = pendingFlushRef.current; + pendingFlushRef.current = null; + if (pending === null || pending === lastEmittedRef.current) return; + lastEmittedRef.current = pending; + onUpdateRef.current?.(pending); }; }, []); diff --git a/packages/views/issues/components/issue-detail.test.tsx b/packages/views/issues/components/issue-detail.test.tsx index 388e63307..5f87a33e6 100644 --- a/packages/views/issues/components/issue-detail.test.tsx +++ b/packages/views/issues/components/issue-detail.test.tsx @@ -129,7 +129,7 @@ vi.mock("../../editor", () => ({
{content}
), ContentEditor: forwardRef(function MockContentEditor( - { defaultValue, onUpdate, placeholder }: any, + { defaultValue, onUpdate, placeholder, flushPendingOnUnmount }: any, ref: any, ) { const valueRef = useRef(defaultValue || ""); @@ -150,6 +150,7 @@ vi.mock("../../editor", () => ({ }} placeholder={placeholder} data-testid="rich-text-editor" + data-flush-on-unmount={flushPendingOnUnmount ? "true" : undefined} /> ); }), @@ -535,6 +536,19 @@ describe("IssueDetail (shared)", () => { expect(screen.getByDisplayValue("Add JWT auth to the backend")).toBeInTheDocument(); }); + it("opts the description editor into the unmount flush", async () => { + // Closing the issue modal must save the description the user last saw — + // ContentEditor drops pending debounced updates on unmount by default + // (so cancelled comment drafts aren't resurrected), and only this + // explicit opt-in keeps a paste-then-close from losing the image + // markdown and its attachment_ids bind (MUL-3254). The flush behavior + // itself is covered in content-editor.test.tsx; this pins the wiring. + renderIssueDetail(); + + const description = await screen.findByDisplayValue("Add JWT auth to the backend"); + expect(description).toHaveAttribute("data-flush-on-unmount", "true"); + }); + it("renders the issue title leaf as a link to the issue detail page", async () => { renderIssueDetail(); diff --git a/packages/views/issues/components/issue-detail.tsx b/packages/views/issues/components/issue-detail.tsx index 77b043792..60c76105c 100644 --- a/packages/views/issues/components/issue-detail.tsx +++ b/packages/views/issues/components/issue-detail.tsx @@ -1198,18 +1198,30 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr // `attachment_ids` on the next description save. Drives editor previews // so text/code attachments show an Eye before the bind round-trips. const [descPendingAttachments, setDescPendingAttachments] = useState([]); + const descPendingAttachmentsRef = useRef([]); const descEditorAttachments = descPendingAttachments.length > 0 ? [...(issueAttachments ?? []), ...descPendingAttachments] : issueAttachments; const handleDescriptionUpload = useCallback( async (file: File) => { const result = await uploadWithToast(file); - if (result) setDescPendingAttachments((prev) => [...prev, result]); + if (result) { + descPendingAttachmentsRef.current = [ + ...descPendingAttachmentsRef.current, + result, + ]; + setDescPendingAttachments(descPendingAttachmentsRef.current); + } return result; }, [uploadWithToast], ); + useEffect(() => { + descPendingAttachmentsRef.current = []; + setDescPendingAttachments([]); + }, [id]); + // Shared issue actions (mutations, pin, copy-link, modal dispatch, etc.). // Called before the `if (!issue)` early return so hook order stays stable. const actions = useIssueActions(issue); @@ -1862,13 +1874,17 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr // webview) — while still working on web via the cookie/proxy. // This mirrors the comment/reply/chat composers, which already // bind via `contentReferencesAttachment` (MUL-3130 / MUL-3192). - const ids = descPendingAttachments + const ids = descPendingAttachmentsRef.current .filter((a) => contentReferencesAttachment(md, a)) .map((a) => a.id); handleUpdateField({ description: md, attachment_ids: ids.length > 0 ? ids : undefined }); }} onUploadFile={handleDescriptionUpload} debounceMs={1500} + // Closing the issue modal must save what the user last saw — + // without the flush, a paste followed by a quick close loses + // the image markdown and its attachment_ids bind (MUL-3254). + flushPendingOnUnmount currentIssueId={id} attachments={descEditorAttachments} />