From e22829f233162966d19da462aab8a93c7bb26efc Mon Sep 17 00:00:00 2001 From: lexflow-agent Date: Wed, 16 Sep 2026 12:55:28 +0000 Subject: [PATCH] fix(editor): flush pending autosave and keep doc switches history-safe Remount TipTap per docId so load is not an undoable setContent, flush on tab hide/close, show a three-state save indicator, and toast failed exports. Co-authored-by: Cursor --- frontend/src/i18n/locales/en/common.json | 4 + frontend/src/i18n/locales/es/common.json | 4 + frontend/src/pages/EditorPage.test.tsx | 142 +++++++++++++++--- frontend/src/pages/EditorPage.tsx | 137 +++++++++++------ frontend/src/pages/editor/ExportMenu.test.tsx | 67 +++++++++ frontend/src/pages/editor/ExportMenu.tsx | 9 +- .../pages/editor/extensions/CitationChip.tsx | 5 +- 7 files changed, 301 insertions(+), 67 deletions(-) create mode 100644 frontend/src/pages/editor/ExportMenu.test.tsx diff --git a/frontend/src/i18n/locales/en/common.json b/frontend/src/i18n/locales/en/common.json index 71635c28..8010fc9b 100644 --- a/frontend/src/i18n/locales/en/common.json +++ b/frontend/src/i18n/locales/en/common.json @@ -52,6 +52,10 @@ "documentTitleAria": "Document title", "placeholder": "Untitled document", "savedAt": "Saved {{time}}", + "saveStatusSaving": "Saving…", + "saveStatusUnsaved": "Unsaved", + "exportErrorTitle": "Could not export", + "exportErrorBody": "Try again or export as Markdown.", "contentPlaceholder": "Start writing your document…", "toolbar": { "aria": "Editor toolbar", diff --git a/frontend/src/i18n/locales/es/common.json b/frontend/src/i18n/locales/es/common.json index 13309fb4..ad39f8e2 100644 --- a/frontend/src/i18n/locales/es/common.json +++ b/frontend/src/i18n/locales/es/common.json @@ -52,6 +52,10 @@ "documentTitleAria": "Título del documento", "placeholder": "Documento sin título", "savedAt": "Guardado {{time}}", + "saveStatusSaving": "Guardando…", + "saveStatusUnsaved": "Sin guardar", + "exportErrorTitle": "No se pudo exportar", + "exportErrorBody": "Vuelve a intentarlo o exporta en Markdown.", "contentPlaceholder": "Empieza a escribir tu documento…", "toolbar": { "aria": "Barra de herramientas del editor", diff --git a/frontend/src/pages/EditorPage.test.tsx b/frontend/src/pages/EditorPage.test.tsx index 6f38f6a6..1332cdfb 100644 --- a/frontend/src/pages/EditorPage.test.tsx +++ b/frontend/src/pages/EditorPage.test.tsx @@ -1,7 +1,7 @@ import { useEffect } from 'react'; -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import { MemoryRouter, Route, Routes, useNavigate } from 'react-router-dom'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const { saveDocumentMock, createDocumentMock, getDocumentMock } = vi.hoisted(() => ({ saveDocumentMock: vi.fn(), @@ -104,6 +104,22 @@ function renderEditor(initialPath = '/editor/draft') { return renderEditorAtDoc(docId); } +function storedDoc(overrides?: { id?: string; updatedAt?: string }) { + return { + id: overrides?.id ?? 'draft', + title: 'Untitled', + content: { type: 'doc', content: [{ type: 'paragraph' }] }, + updatedAt: overrides?.updatedAt ?? '2026-09-12T12:00:00.000Z', + }; +} + +function typePendingContent(content: ReturnType<(typeof mockEditor)['getJSON']>) { + mockEditor.getJSON.mockReturnValue(content); + act(() => { + onUpdateHandler?.({ editor: mockEditor }); + }); +} + describe('EditorPage autosave flush', () => { beforeEach(() => { saveDocumentMock.mockReset(); @@ -116,6 +132,7 @@ describe('EditorPage autosave flush', () => { type: 'doc', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'saved' }] }], }); + mockEditor.commands.setContent.mockReset(); onUpdateHandler = undefined; }); @@ -126,8 +143,7 @@ describe('EditorPage autosave flush', () => { }; const view = renderEditorAtDoc('doc-a'); expect(onUpdateHandler).toBeDefined(); - mockEditor.getJSON.mockReturnValue(pendingContent); - onUpdateHandler?.({ editor: mockEditor }); + typePendingContent(pendingContent); expect(saveDocumentMock).not.toHaveBeenCalled(); view.rerender( @@ -150,6 +166,9 @@ describe('EditorPage autosave flush', () => { expect(saveDocumentMock).not.toHaveBeenCalledWith( expect.objectContaining({ id: 'doc-b', content: pendingContent }), ); + // Incoming doc loads via a remounted useEditor({ content }), not setContent. + expect(mockEditor.commands.setContent).not.toHaveBeenCalled(); + expect(saveDocumentMock).not.toHaveBeenCalledWith(expect.objectContaining({ id: 'doc-b' })); }); it('creates a document from the picker and navigates to the new id', async () => { @@ -174,8 +193,7 @@ describe('EditorPage autosave flush', () => { }; const view = renderEditor(); expect(onUpdateHandler).toBeDefined(); - mockEditor.getJSON.mockReturnValue(pendingContent); - onUpdateHandler?.({ editor: mockEditor }); + typePendingContent(pendingContent); expect(saveDocumentMock).not.toHaveBeenCalled(); view.unmount(); await waitFor(() => { @@ -187,13 +205,63 @@ describe('EditorPage autosave flush', () => { }); }); - it('uses Spanish i18n for title aria, placeholder and saved line', () => { - getDocumentMock.mockReturnValue({ + it('flushes pending save when the tab is hidden', () => { + const pendingContent = { + type: 'doc', + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'hidden-tab' }] }], + }; + renderEditor(); + typePendingContent(pendingContent); + expect(saveDocumentMock).not.toHaveBeenCalled(); + + const originalVisibility = document.visibilityState; + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: () => 'hidden', + }); + act(() => { + document.dispatchEvent(new Event('visibilitychange')); + }); + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: () => originalVisibility, + }); + + expect(saveDocumentMock).toHaveBeenCalledWith({ id: 'draft', - title: 'Untitled', - content: { type: 'doc', content: [{ type: 'paragraph' }] }, - updatedAt: '2026-09-12T12:00:00.000Z', + title: 'Draft', + content: pendingContent, }); + }); + + it('registers a beforeunload listener that flushes pending save', () => { + const pendingContent = { + type: 'doc', + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'leaving' }] }], + }; + renderEditor(); + typePendingContent(pendingContent); + expect(saveDocumentMock).not.toHaveBeenCalled(); + + act(() => { + window.dispatchEvent(new Event('beforeunload')); + }); + + expect(saveDocumentMock).toHaveBeenCalledWith({ + id: 'draft', + title: 'Draft', + content: pendingContent, + }); + }); + + it('does not call setContent or save on initial document load', () => { + renderEditor(); + expect(mockEditor.commands.setContent).not.toHaveBeenCalled(); + expect(saveDocumentMock).not.toHaveBeenCalled(); + }); + + it('uses Spanish i18n for title aria, placeholder and saved line', () => { + getDocumentMock.mockReturnValue(storedDoc()); renderEditor(); expect(screen.getByLabelText('Título del documento')).toBeInTheDocument(); expect(screen.getByPlaceholderText('Documento sin título')).toBeInTheDocument(); @@ -203,12 +271,7 @@ describe('EditorPage autosave flush', () => { it('uses English i18n for title aria and placeholder when locale is en', async () => { i18n.addResourceBundle('en', 'common', en, true, true); await i18n.changeLanguage('en'); - getDocumentMock.mockReturnValue({ - id: 'draft', - title: 'Untitled', - content: { type: 'doc', content: [{ type: 'paragraph' }] }, - updatedAt: '2026-09-12T12:00:00.000Z', - }); + getDocumentMock.mockReturnValue(storedDoc()); renderEditor(); expect(screen.getByLabelText('Document title')).toBeInTheDocument(); expect(screen.getByPlaceholderText('Untitled document')).toBeInTheDocument(); @@ -216,3 +279,48 @@ describe('EditorPage autosave flush', () => { await i18n.changeLanguage('es'); }); }); + +describe('EditorPage save indicator', () => { + beforeEach(() => { + vi.useFakeTimers(); + saveDocumentMock.mockReset(); + getDocumentMock.mockReset(); + getDocumentMock.mockReturnValue(storedDoc()); + mockEditor.getJSON.mockReset(); + mockEditor.getJSON.mockReturnValue({ + type: 'doc', + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'saved' }] }], + }); + onUpdateHandler = undefined; + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('shows unsaved while debounce is pending, then saved after flush', () => { + renderEditor(); + expect(screen.getByTestId('editor-save-status')).toHaveTextContent(/^Guardado /); + + typePendingContent({ + type: 'doc', + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'dirty' }] }], + }); + expect(screen.getByTestId('editor-save-status')).toHaveTextContent('Sin guardar'); + + act(() => { + vi.advanceTimersByTime(600); + }); + expect(saveDocumentMock).toHaveBeenCalled(); + expect(screen.getByTestId('editor-save-status')).toHaveTextContent(/^Guardado /); + }); + + it('returns to saved immediately when the title is edited', () => { + renderEditor(); + fireEvent.change(screen.getByLabelText('Título del documento'), { + target: { value: 'Nuevo título' }, + }); + expect(saveDocumentMock).toHaveBeenCalled(); + expect(screen.getByTestId('editor-save-status')).toHaveTextContent(/^Guardado /); + }); +}); diff --git a/frontend/src/pages/EditorPage.tsx b/frontend/src/pages/EditorPage.tsx index 45303c83..01eb4380 100644 --- a/frontend/src/pages/EditorPage.tsx +++ b/frontend/src/pages/EditorPage.tsx @@ -13,11 +13,15 @@ * * Invariants: * - The editor is always mounted with `immediatelyRender: true` (CSR only). - * - On unmount the debounce timer is cancelled via the cleanup returned - * from `useEffect` so a queued save never fires against an unmounted - * component. + * - A pending debounce is flushed immediately on unmount, doc switch (the + * keyed surface unmounts), tab hide (`visibilitychange`), and `beforeunload`, + * so the last keystrokes persist instead of being cancelled. * - `docId` is required on `/editor/:docId`. Bare `/editor` renders the * document picker so the user can create or switch documents (#57 S1.4). + * - The TipTap instance remounts per `docId` (`key={docId}`) so each + * document gets a fresh undo stack and loads via `useEditor({ content })` + * without a follow-up `setContent` (which would be undoable and would + * emit `onUpdate`, scheduling a redundant save). * * --- WHERE TO CHANGE IF EDITOR FEATURES CHANGE --- * - Add a new TipTap extension → install it, add to `extensions` below, @@ -51,6 +55,8 @@ import { DocumentList, DocumentPicker } from '@/pages/editor/DocumentList'; /** Debounce window before a content change is written to localStorage (ms). */ const AUTOSAVE_DELAY_MS = 600; +type SaveStatus = 'saved' | 'saving' | 'unsaved'; + /** Cancel any pending debounce and write the latest editor state immediately (#44 R5). */ function flushPendingAutosave( autosaveTimer: MutableRefObject | null>, @@ -60,16 +66,23 @@ function flushPendingAutosave( saveDocument: (doc: { id: string; title: string; content: ReturnType }) => void, /** When switching docs, pass the outgoing id — `docIdRef` may already point at the new route. */ docIdOverride?: string, -): void { - if (autosaveTimer.current === null) return; +): boolean { + if (autosaveTimer.current === null) return false; clearTimeout(autosaveTimer.current); autosaveTimer.current = null; - if (!editor) return; + if (!editor) return false; saveDocument({ id: docIdOverride ?? docIdRef.current, title: titleRef.current, content: editor.getJSON(), }); + return true; +} + +function clearAutosaveTimer(autosaveTimer: MutableRefObject | null>): void { + if (autosaveTimer.current === null) return; + clearTimeout(autosaveTimer.current); + autosaveTimer.current = null; } /** @@ -86,7 +99,35 @@ function intlLocale(language: string): string { return language.startsWith('en') ? 'en-GB' : 'es-ES'; } +/** + * Shell around a single document: sidebar list stays mounted across + * switches; the TipTap surface remounts per `docId` (S1.2). + */ function EditorWorkspace({ docId }: { docId: string }) { + return ( +
+ +
+
+ +
+ +
+
+ ); +} + +/** + * Per-document TipTap instance, autosave lifecycle, toolbar, and panels. + * + * Mounted with `key={docId}` so switching documents destroys the previous + * editor (flushing a pending debounce via unmount cleanup) and creates a + * new one with the incoming JSON as initial `content` — no `setContent`, + * no undo history bleed, no save-on-load. + */ +function EditorDocumentSurface({ docId }: { docId: string }) { const { t, i18n } = useTranslation(); const { getDocument, saveDocument, persistError, clearPersistError } = useEditorStore(); @@ -115,7 +156,9 @@ function EditorWorkspace({ docId }: { docId: string }) { const [focusCommentId, setFocusCommentId] = useState(null); const addComment = useCommentStore((s) => s.addComment); - // Ref to hold the active autosave timeout so it can be cancelled on unmount. + const [saveStatus, setSaveStatus] = useState('saved'); + + // Ref to hold the active autosave timeout so it can be flushed on unmount. const autosaveTimer = useRef | null>(null); // `useEditor`'s `onUpdate` closes over the values at creation time, so the @@ -149,16 +192,19 @@ function EditorWorkspace({ docId }: { docId: string }) { editable: !isReadOnly, immediatelyRender: true, onUpdate: ({ editor: ed }) => { - // Debounce: cancel the previous timer and start a fresh one. if (autosaveTimer.current !== null) { clearTimeout(autosaveTimer.current); } + setSaveStatus('unsaved'); autosaveTimer.current = setTimeout(() => { + autosaveTimer.current = null; + setSaveStatus('saving'); saveDocument({ id: docIdRef.current, title: titleRef.current, content: ed.getJSON(), }); + setSaveStatus('saved'); }, AUTOSAVE_DELAY_MS); }, }); @@ -170,6 +216,26 @@ function EditorWorkspace({ docId }: { docId: string }) { }; }, [editor, saveDocument]); + // Tab close / background: same flush as unmount, without cancelling the timer. + useEffect(() => { + const flushIfPending = () => + flushPendingAutosave(autosaveTimer, editor, docIdRef, titleRef, saveDocument); + + const onBeforeUnload = () => { + flushIfPending(); + }; + const onVisibilityChange = () => { + if (document.visibilityState !== 'hidden') return; + if (flushIfPending()) setSaveStatus('saved'); + }; + window.addEventListener('beforeunload', onBeforeUnload); + document.addEventListener('visibilitychange', onVisibilityChange); + return () => { + window.removeEventListener('beforeunload', onBeforeUnload); + document.removeEventListener('visibilitychange', onVisibilityChange); + }; + }, [editor, saveDocument]); + // Sticky warning when localStorage quota is exceeded — export is the escape hatch. useEffect(() => { if (!persistError) return; @@ -181,23 +247,6 @@ function EditorWorkspace({ docId }: { docId: string }) { }); }, [persistError, t]); - // When `docId` changes (the user navigates to a different doc), load the - // correct content and reset the title. - useEffect(() => { - const outgoingDocId = docIdRef.current; - if (!editor) { - docIdRef.current = docId; - return; - } - // Save the outgoing doc before syncing `docIdRef` to the new route param. - flushPendingAutosave(autosaveTimer, editor, docIdRef, titleRef, saveDocument, outgoingDocId); - docIdRef.current = docId; - const doc = getDocument(docId) ?? makeDefaultDocument(docId); - setTitle(doc.title); - // `setContent` resets the editor state to the new JSON document. - editor.commands.setContent(doc.content); - }, [docId, editor, getDocument, saveDocument]); - // Sync the `editable` flag whenever the toggle changes. useEffect(() => { if (!editor) return; @@ -219,11 +268,13 @@ function EditorWorkspace({ docId }: { docId: string }) { const newTitle = e.target.value; setTitle(newTitle); if (!editor) return; + clearAutosaveTimer(autosaveTimer); saveDocument({ id: docId, title: newTitle, content: editor.getJSON(), }); + setSaveStatus('saved'); }, [docId, editor, saveDocument] ); @@ -255,14 +306,7 @@ function EditorWorkspace({ docId }: { docId: string }) { }, [editor, title]); return ( -
- -
-
- -
+ <> {persistError && (
)} - {/* Page header: editable title + doc id breadcrumb, export on the right */} + {/* Page header: editable title + save status, export on the right */}
- - {stored?.updatedAt && ( - <> - {t('editor.savedAt', { - time: new Date(stored.updatedAt).toLocaleTimeString(intlLocale(i18n.language), { - hour: '2-digit', - minute: '2-digit', - }), - })} - - )} + + {saveStatus === 'saving' && t('editor.saveStatusSaving')} + {saveStatus === 'unsaved' && t('editor.saveStatusUnsaved')} + {saveStatus === 'saved' && + stored?.updatedAt && + t('editor.savedAt', { + time: new Date(stored.updatedAt).toLocaleTimeString(intlLocale(i18n.language), { + hour: '2-digit', + minute: '2-digit', + }), + })}
{editor && } @@ -421,7 +465,6 @@ function EditorWorkspace({ docId }: { docId: string }) { >
-
- + ); } diff --git a/frontend/src/pages/editor/ExportMenu.test.tsx b/frontend/src/pages/editor/ExportMenu.test.tsx new file mode 100644 index 00000000..e33344af --- /dev/null +++ b/frontend/src/pages/editor/ExportMenu.test.tsx @@ -0,0 +1,67 @@ +/** + * ExportMenu — failed .docx export must surface a danger toast (S1.4). + */ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Editor } from '@tiptap/react'; + +const exportDocx = vi.fn(); +const exportMarkdown = vi.fn(); +const toast = vi.fn(); + +vi.mock('./export-utils', () => ({ + exportDocx: (...args: unknown[]) => exportDocx(...args), + exportMarkdown: (...args: unknown[]) => exportMarkdown(...args), +})); + +vi.mock('@/lib/toast', () => ({ + toast: (...args: unknown[]) => toast(...args), +})); + +import { ExportMenu } from './ExportMenu'; + +const editor = { + getJSON: () => ({ type: 'doc', content: [] }), +} as unknown as Editor; + +describe('ExportMenu', () => { + beforeEach(() => { + exportDocx.mockReset(); + exportMarkdown.mockReset(); + toast.mockReset(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('shows a danger toast when docx export fails', async () => { + exportDocx.mockRejectedValue(new Error('fail')); + render(); + + fireEvent.click(screen.getByRole('button', { name: /exportar/i })); + fireEvent.click(screen.getByRole('menuitem', { name: /word/i })); + + await waitFor(() => { + expect(toast).toHaveBeenCalledWith({ + tone: 'danger', + title: 'No se pudo exportar', + message: 'Vuelve a intentarlo o exporta en Markdown.', + }); + }); + }); + + it('does not toast when markdown export succeeds', async () => { + exportMarkdown.mockResolvedValue(undefined); + render(); + + fireEvent.click(screen.getByRole('button', { name: /exportar/i })); + fireEvent.click(screen.getByRole('menuitem', { name: /markdown/i })); + + await waitFor(() => { + expect(exportMarkdown).toHaveBeenCalled(); + }); + expect(toast).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/pages/editor/ExportMenu.tsx b/frontend/src/pages/editor/ExportMenu.tsx index 7397019c..efbd4ed4 100644 --- a/frontend/src/pages/editor/ExportMenu.tsx +++ b/frontend/src/pages/editor/ExportMenu.tsx @@ -10,9 +10,11 @@ * dynamic-imports `docx`, so opening this menu costs nothing until used. */ import { useEffect, useRef, useState } from 'react'; +import { useTranslation } from 'react-i18next'; import { Download, FileText, FileType2, ChevronDown } from 'lucide-react'; import type { Editor } from '@tiptap/react'; import { Button } from '@/components/ui'; +import { toast } from '@/lib/toast'; import { exportDocx, exportMarkdown } from './export-utils'; interface ExportMenuProps { @@ -22,6 +24,7 @@ interface ExportMenuProps { } export function ExportMenu({ editor, title }: ExportMenuProps) { + const { t } = useTranslation(); const [open, setOpen] = useState(false); const rootRef = useRef(null); @@ -47,8 +50,12 @@ export function ExportMenu({ editor, title }: ExportMenuProps) { try { await action(); } catch (err) { - // Export is a user action; a silent failure would look like a no-op. console.error('Document export failed', err); + toast({ + tone: 'danger', + title: t('editor.exportErrorTitle'), + message: t('editor.exportErrorBody'), + }); } } diff --git a/frontend/src/pages/editor/extensions/CitationChip.tsx b/frontend/src/pages/editor/extensions/CitationChip.tsx index fc4303ec..c8e91a30 100644 --- a/frontend/src/pages/editor/extensions/CitationChip.tsx +++ b/frontend/src/pages/editor/extensions/CitationChip.tsx @@ -21,8 +21,9 @@ export function CitationChip({ node }: NodeViewProps) { const goToSource = () => { if (!lawId) return; - // ponytail: navigate on click in any mode — the draft is autosaved, so - // jumping to the source can't lose work. In-place re-pick deferred (#599). + // Autosave is debounced (~600 ms), not instant. Unmount flush plus tab + // hide/close handlers persist keystrokes inside that window when this + // click navigates away. In-place re-pick deferred (#599). navigate(`/laws/${encodeURIComponent(lawId)}`); };