diff --git a/packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterRegistry.test.ts b/packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterRegistry.test.ts index c7af0a33ee..ac2958d450 100644 --- a/packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterRegistry.test.ts +++ b/packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterRegistry.test.ts @@ -67,14 +67,23 @@ const { mockCreateHeaderFooterEditor, mockOnHeaderFooterDataUpdate, mockToFlowBl return editorStub; }; - const mockCreateHeaderFooterEditor = vi.fn(() => { - const editor = createSectionEditor(); - editors.push({ editor, emit: editor.emit }); - queueMicrotask(() => { - editor.emit('create'); - }); - return editor; - }); + const mockCreateHeaderFooterEditor = vi.fn( + (input?: { editorContainer?: HTMLElement; editorHost?: HTMLElement }) => { + const editor = createSectionEditor(); + if (input?.editorContainer instanceof HTMLElement) { + if (input.editorHost instanceof HTMLElement) { + input.editorHost.appendChild(input.editorContainer); + } else { + document.body.appendChild(input.editorContainer); + } + } + editors.push({ editor, emit: editor.emit }); + queueMicrotask(() => { + editor.emit('create'); + }); + return editor; + }, + ); return { mockCreateHeaderFooterEditor, @@ -192,6 +201,39 @@ describe('HeaderFooterEditorManager', () => { ); }); + it('ensureEditorSync creates a reusable editor instance immediately for presentation activation', () => { + const editor = createMockEditor(); + const manager = new HeaderFooterEditorManager(editor); + const descriptor = { id: 'rId-header-default', kind: 'header' } as const; + const host = document.createElement('div'); + + const first = manager.ensureEditorSync(descriptor, { editorHost: host }); + const second = manager.ensureEditorSync(descriptor, { editorHost: host }); + + expect(first).toBeDefined(); + expect(second).toBe(first); + expect(mockCreateHeaderFooterEditor).toHaveBeenCalledTimes(1); + expect(host.children).toHaveLength(1); + }); + + it('ensureEditorSync reattaches the cached editor container to a new host', () => { + const editor = createMockEditor(); + const manager = new HeaderFooterEditorManager(editor); + const descriptor = { id: 'rId-header-default', kind: 'header' } as const; + const firstHost = document.createElement('div'); + const secondHost = document.createElement('div'); + + const sectionEditor = manager.ensureEditorSync(descriptor, { editorHost: firstHost }); + expect(sectionEditor).toBeDefined(); + expect(firstHost.children).toHaveLength(1); + + const sameEditor = manager.ensureEditorSync(descriptor, { editorHost: secondHost }); + + expect(sameEditor).toBe(sectionEditor); + expect(firstHost.children).toHaveLength(0); + expect(secondHost.children).toHaveLength(1); + }); + it('emits contentChanged and syncs converter/Yjs data when section editor updates', async () => { const editor = createMockEditor(); const manager = new HeaderFooterEditorManager(editor); diff --git a/packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterRegistry.ts b/packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterRegistry.ts index 8c61429e08..4ad46e389c 100644 --- a/packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterRegistry.ts +++ b/packages/super-editor/src/editors/v1/core/header-footer/HeaderFooterRegistry.ts @@ -330,39 +330,7 @@ export class HeaderFooterEditorManager extends EventEmitter { console.error('[HeaderFooterEditorManager] Editor initialization failed:', error); this.emit('error', { descriptor, error }); }); - - // Move editor container to the new editorHost if provided - // This is necessary because cached editors may have been appended elsewhere - if (existing.container && options?.editorHost) { - // Only move if not already in the target host - if (existing.container.parentElement !== options.editorHost) { - options.editorHost.appendChild(existing.container); - } - } - - // Update editor options if provided - if (existing.editor && options) { - const updateOptions: Record = {}; - if (options.currentPageNumber !== undefined) { - updateOptions.currentPageNumber = options.currentPageNumber; - } - if (options.totalPageCount !== undefined) { - updateOptions.totalPageCount = options.totalPageCount; - } - if (options.availableWidth !== undefined) { - updateOptions.availableWidth = options.availableWidth; - } - if (options.availableHeight !== undefined) { - updateOptions.availableHeight = options.availableHeight; - } - if (Object.keys(updateOptions).length > 0) { - existing.editor.setOptions(updateOptions); - // Refresh page number display after option changes. - // NodeViews read editor.options but PM doesn't re-render them - // when only options change (no document transaction). - this.#refreshPageNumberDisplay(existing.editor); - } - } + this.#mountAndUpdateEntry(existing, options); return existing.editor; } @@ -380,7 +348,7 @@ export class HeaderFooterEditorManager extends EventEmitter { // Start creation and track the promise const creationPromise = (async () => { try { - const entry = await this.#createEditor(descriptor, options); + const entry = this.#createEditorEntry(descriptor, options); if (!entry) return null; this.#editorEntries.set(descriptor.id, entry); @@ -406,6 +374,44 @@ export class HeaderFooterEditorManager extends EventEmitter { return creationPromise; } + /** + * Synchronously returns the cached editor for a descriptor, creating it on demand. + * + * Presentation-mode story activation needs a stable editor instance and DOM + * target immediately so input can be forwarded into the hidden host without + * waiting for the async `create` event. The normal lifecycle hooks still run + * through the returned entry's `ready` promise. + */ + ensureEditorSync( + descriptor: HeaderFooterDescriptor, + options?: { + editorHost?: HTMLElement; + availableWidth?: number; + availableHeight?: number; + currentPageNumber?: number; + totalPageCount?: number; + }, + ): Editor | null { + if (!descriptor?.id) return null; + + const existing = this.#editorEntries.get(descriptor.id); + if (existing) { + this.#cacheHits += 1; + this.#updateAccessOrder(descriptor.id); + this.#mountAndUpdateEntry(existing, options); + return existing.editor; + } + + const entry = this.#createEditorEntry(descriptor, options); + if (!entry) return null; + + this.#cacheMisses += 1; + this.#editorEntries.set(descriptor.id, entry); + this.#updateAccessOrder(descriptor.id); + this.#enforceCacheSizeLimit(); + return entry.editor; + } + /** * Updates page number DOM elements to reflect current editor options. * Called after setOptions to sync NodeViews that read editor.options. @@ -671,7 +677,7 @@ export class HeaderFooterEditorManager extends EventEmitter { this.#editorEntries.clear(); } - async #createEditor( + #createEditorEntry( descriptor: HeaderFooterDescriptor, options?: { editorHost?: HTMLElement; @@ -680,7 +686,7 @@ export class HeaderFooterEditorManager extends EventEmitter { currentPageNumber?: number; totalPageCount?: number; }, - ): Promise { + ): HeaderFooterEditorEntry | null { const json = this.getDocumentJson(descriptor); if (!json) return null; @@ -799,6 +805,45 @@ export class HeaderFooterEditorManager extends EventEmitter { }; } + #mountAndUpdateEntry( + entry: HeaderFooterEditorEntry, + options?: { + editorHost?: HTMLElement; + availableWidth?: number; + availableHeight?: number; + currentPageNumber?: number; + totalPageCount?: number; + }, + ): void { + if (entry.container && options?.editorHost && entry.container.parentElement !== options.editorHost) { + options.editorHost.appendChild(entry.container); + } + + if (!options) { + return; + } + + const updateOptions: Record = {}; + if (options.currentPageNumber !== undefined) { + updateOptions.currentPageNumber = options.currentPageNumber; + } + if (options.totalPageCount !== undefined) { + updateOptions.totalPageCount = options.totalPageCount; + } + if (options.availableWidth !== undefined) { + updateOptions.availableWidth = options.availableWidth; + } + if (options.availableHeight !== undefined) { + updateOptions.availableHeight = options.availableHeight; + } + if (Object.keys(updateOptions).length > 0) { + entry.editor.setOptions(updateOptions); + // NodeViews that render PAGE / NUMPAGES read editor.options, so refresh + // them when the presentation context changes without a document step. + this.#refreshPageNumberDisplay(entry.editor); + } + } + #createEditorContainer(): HTMLElement { const doc = (this.#editor.options?.element?.ownerDocument as Document | undefined) ?? globalThis.document ?? undefined; diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts index d799524e98..19319e9114 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts @@ -125,6 +125,7 @@ import type { import { extractHeaderFooterSpace as _extractHeaderFooterSpace } from '@superdoc/contracts'; // TrackChangesBasePluginKey is used by #syncTrackedChangesPreferences and getTrackChangesPluginState. import { TrackChangesBasePluginKey } from '@extensions/track-changes/plugins/index.js'; +import { runEditorRedo, runEditorUndo } from '@extensions/history/history.js'; // Collaboration cursor imports import { ySyncPluginKey } from 'y-prosemirror'; @@ -170,6 +171,45 @@ type NoteLayoutContext = { hostWidthPx: number; }; +const VOLATILE_HISTORY_ATTR_KEYS = new Set(['sdBlockId', 'sdBlockRev']); + +function stripVolatileHistoryAttrs(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map((item) => stripVolatileHistoryAttrs(item)); + } + + if (!value || typeof value !== 'object') { + return value; + } + + const result: Record = {}; + for (const [key, entryValue] of Object.entries(value as Record)) { + if (VOLATILE_HISTORY_ATTR_KEYS.has(key)) { + continue; + } + result[key] = stripVolatileHistoryAttrs(entryValue); + } + return result; +} + +function docsEqualIgnoringVolatileHistoryAttrs( + before: ProseMirrorNode | null | undefined, + after: ProseMirrorNode | null | undefined, +): boolean { + if (!before || !after) { + return false; + } + + if (typeof before.eq === 'function' && before.eq(after)) { + return true; + } + + const beforeJson = typeof before.toJSON === 'function' ? before.toJSON() : before; + const afterJson = typeof after.toJSON === 'function' ? after.toJSON() : after; + + return JSON.stringify(stripVolatileHistoryAttrs(beforeJson)) === JSON.stringify(stripVolatileHistoryAttrs(afterJson)); +} + type RenderedNoteFragmentHit = { fragmentElement: HTMLElement; pageIndex: number; @@ -473,6 +513,8 @@ export class PresentationEditor extends EventEmitter { #storySessionSelectionHandler: ((...args: unknown[]) => void) | null = null; #storySessionTransactionHandler: ((...args: unknown[]) => void) | null = null; #storySessionEditor: Editor | null = null; + #persistentStorySessionEditors = new WeakSet(); + #lastPersistentStoryHistoryEditor: Editor | null = null; #activeSurfaceUiEventEditor: Editor | null = null; #activeSurfaceUiUpdateHandler: ((...args: unknown[]) => void) | null = null; #activeSurfaceUiContextMenuOpenHandler: ((...args: unknown[]) => void) | null = null; @@ -1442,13 +1484,119 @@ export class PresentationEditor extends EventEmitter { }; } + #runEditorHistoryCommand( + editor: Editor | null, + command: 'undo' | 'redo', + ): { didRun: boolean; didChangeDoc: boolean } { + if (!editor) { + return { didRun: false, didChangeDoc: false }; + } + + const beforeDoc = editor.state?.doc ?? null; + + try { + const didRun = command === 'undo' ? runEditorUndo(editor) : runEditorRedo(editor); + const rawDidChangeDoc = + beforeDoc && editor.state?.doc && typeof editor.state.doc.eq === 'function' + ? !editor.state.doc.eq(beforeDoc) + : didRun; + const didChangeDoc = + editor === this.#editor && + rawDidChangeDoc && + docsEqualIgnoringVolatileHistoryAttrs(beforeDoc, editor.state?.doc) + ? false + : rawDidChangeDoc; + + if (didRun && this.#persistentStorySessionEditors.has(editor)) { + this.#lastPersistentStoryHistoryEditor = editor; + } + + return { didRun, didChangeDoc }; + } catch { + return { didRun: false, didChangeDoc: false }; + } + } + + #runPersistentStoryHistoryCommand(command: 'undo' | 'redo'): boolean { + const editor = this.#lastPersistentStoryHistoryEditor; + if (!editor || !this.#persistentStorySessionEditors.has(editor)) { + return false; + } + + const handler = command === 'undo' ? editor.commands?.undo : editor.commands?.redo; + if (typeof handler !== 'function') { + return false; + } + + try { + const didRun = Boolean(handler()); + if (didRun) { + this.#lastPersistentStoryHistoryEditor = editor; + } + return didRun; + } catch { + return false; + } + } + + #canRunEditorHistoryCommand(editor: Editor | null, command: 'undo' | 'redo'): boolean { + if (!editor) { + return false; + } + + try { + return Boolean( + command === 'undo' + ? runEditorUndo(editor, { allowDispatch: false }) + : runEditorRedo(editor, { allowDispatch: false }), + ); + } catch { + return false; + } + } + + #canRunPersistentStoryHistoryCommand(command: 'undo' | 'redo'): boolean { + const editor = this.#lastPersistentStoryHistoryEditor; + if (!editor || !this.#persistentStorySessionEditors.has(editor)) { + return false; + } + + return this.#canRunEditorHistoryCommand(editor, command); + } + + canUndo(): boolean { + const editor = this.getActiveEditor(); + if (this.#canRunEditorHistoryCommand(editor, 'undo')) { + return true; + } + if (editor === this.#editor) { + return this.#canRunPersistentStoryHistoryCommand('undo'); + } + return false; + } + + canRedo(): boolean { + const editor = this.getActiveEditor(); + if (this.#canRunEditorHistoryCommand(editor, 'redo')) { + return true; + } + if (editor === this.#editor) { + return this.#canRunPersistentStoryHistoryCommand('redo'); + } + return false; + } + /** * Undo the last action in the active editor. */ undo(): boolean { const editor = this.getActiveEditor(); - if (editor?.commands?.undo) { - return Boolean(editor.commands.undo()); + const { didRun, didChangeDoc } = this.#runEditorHistoryCommand(editor, 'undo'); + if (didRun && (editor !== this.#editor || didChangeDoc)) { + return true; + } + if (editor === this.#editor) { + return this.#runPersistentStoryHistoryCommand('undo'); } return false; } @@ -1458,8 +1606,12 @@ export class PresentationEditor extends EventEmitter { */ redo(): boolean { const editor = this.getActiveEditor(); - if (editor?.commands?.redo) { - return Boolean(editor.commands.redo()); + const { didRun, didChangeDoc } = this.#runEditorHistoryCommand(editor, 'redo'); + if (didRun && (editor !== this.#editor || didChangeDoc)) { + return true; + } + if (editor === this.#editor) { + return this.#runPersistentStoryHistoryCommand('redo'); } return false; } @@ -4774,6 +4926,10 @@ export class PresentationEditor extends EventEmitter { return; } + if (this.#persistentStorySessionEditors.has(session.editor)) { + this.#lastPersistentStoryHistoryEditor = session.editor; + } + if (session.kind === 'note') { this.#invalidateTrackedChangesForStory(session.locator); this.#pendingDocChange = true; @@ -4830,9 +4986,28 @@ export class PresentationEditor extends EventEmitter { return doc?.body ?? this.#visibleHost ?? null; }, editorFactory: ({ runtime, hostElement, activationOptions }) => { + const editorContext = activationOptions.editorContext ?? {}; + + if (runtime.kind === 'headerFooter' && runtime.locator.storyType === 'headerFooterPart') { + const descriptor = this.#headerFooterSession?.manager?.getDescriptorById(runtime.locator.refId) ?? null; + const persisted = descriptor + ? (this.#headerFooterSession?.manager?.ensureEditorSync(descriptor, { + editorHost: hostElement, + availableWidth: editorContext.availableWidth, + availableHeight: editorContext.availableHeight, + currentPageNumber: editorContext.currentPageNumber, + totalPageCount: editorContext.totalPageCount, + }) ?? null) + : null; + + if (persisted) { + this.#persistentStorySessionEditors.add(persisted); + return { editor: persisted }; + } + } + const existing = runtime.editor; const pmJson = existing.getJSON() as unknown as Record; - const editorContext = activationOptions.editorContext ?? {}; const fresh = createStoryEditor(this.#editor, pmJson, { documentId: runtime.storyKey, isHeaderOrFooter: runtime.kind === 'headerFooter', diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts index e2368466fb..f5adc628c6 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts @@ -1021,7 +1021,11 @@ export class HeaderFooterSessionManager { const bodyPageCount = this.#deps?.getBodyPageCount() ?? 1; const session = storySessionManager.activate(locator, { - commitPolicy: 'continuous', + // Presentation-mode header/footer sessions now reuse the manager-backed + // per-refId editor, which already exports on update. Commit once on exit + // to avoid double-syncing every keystroke while still flushing the final + // state if the session closes mid-batch. + commitPolicy: 'onExit', preferHiddenHost: true, hostWidthPx: Math.max(1, region.width), editorContext: { @@ -1063,6 +1067,7 @@ export class HeaderFooterSessionManager { sectionId: region.sectionId, kind: region.kind, variant: normalizeVariant(region.sectionType ?? 'default'), + addToHistory: false, }); if (materializationResult) { // Refresh registry so the new refId is discoverable diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts index 63a65b797e..d500d6c5c0 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/HeaderFooterSessionManager.test.ts @@ -388,7 +388,7 @@ describe('HeaderFooterSessionManager', () => { refId: 'rId-header-default', }, expect.objectContaining({ - commitPolicy: 'continuous', + commitPolicy: 'onExit', preferHiddenHost: true, hostWidthPx: 480, editorContext: expect.objectContaining({ diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts index bb532f01e9..f603f5b9c0 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts @@ -2437,8 +2437,12 @@ describe('PresentationEditor', () => { viewport.dispatchEvent(new MouseEvent('dblclick', { bubbles: true, clientX: 120, clientY: 50, button: 0 })); - await vi.waitFor(() => expect(createdStoryEditors.length).toBeGreaterThan(0)); - await vi.waitFor(() => expect(editor.getActiveEditor()).toBe(createdStoryEditors.at(-1)?.editor)); + await vi.waitFor(() => expect(createdSectionEditors.length).toBeGreaterThan(0)); + await vi.waitFor(() => + expect( + createdSectionEditors.some(({ editor: sectionEditor }) => sectionEditor === editor.getActiveEditor()), + ).toBe(true), + ); const sourceEditor = editor.getActiveEditor(); expect(sourceEditor).toBeDefined(); @@ -2505,8 +2509,12 @@ describe('PresentationEditor', () => { viewport.dispatchEvent(new MouseEvent('dblclick', { bubbles: true, clientX: 120, clientY: 50, button: 0 })); - await vi.waitFor(() => expect(createdStoryEditors.length).toBeGreaterThan(0)); - await vi.waitFor(() => expect(editor.getActiveEditor()).toBe(createdStoryEditors.at(-1)?.editor)); + await vi.waitFor(() => expect(createdSectionEditors.length).toBeGreaterThan(0)); + await vi.waitFor(() => + expect( + createdSectionEditors.some(({ editor: sectionEditor }) => sectionEditor === editor.getActiveEditor()), + ).toBe(true), + ); const sourceEditor = editor.getActiveEditor(); const transaction = { docChanged: true }; @@ -2563,8 +2571,12 @@ describe('PresentationEditor', () => { viewport.dispatchEvent(new MouseEvent('dblclick', { bubbles: true, clientX: 120, clientY: 740, button: 0 })); - await vi.waitFor(() => expect(createdStoryEditors.length).toBeGreaterThan(0)); - await vi.waitFor(() => expect(editor.getActiveEditor()).toBe(createdStoryEditors.at(-1)?.editor)); + await vi.waitFor(() => expect(createdSectionEditors.length).toBeGreaterThan(0)); + await vi.waitFor(() => + expect( + createdSectionEditors.some(({ editor: sectionEditor }) => sectionEditor === editor.getActiveEditor()), + ).toBe(true), + ); const sourceEditor = editor.getActiveEditor(); expect(sourceEditor).toBeDefined(); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/header-footer-slot-materialization.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/header-footer-slot-materialization.test.ts index 67d8ccc100..4e9cb0e2c7 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/header-footer-slot-materialization.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/header-footer-slot-materialization.test.ts @@ -182,6 +182,29 @@ describe('ensureExplicitHeaderFooterSlot', () => { expect(mockApplySectPrToProjection).toHaveBeenCalled(); }); + it('can materialize without creating an undoable body history step', () => { + mockSectionProjections.mockReturnValue([createProjection('section-0', 0)]); + mockReadTargetSectPr.mockReturnValue(createSectPr()); + mockResolveEffectiveRef.mockReturnValue(null); + mockCreateHeaderFooterPart.mockReturnValue({ + refId: 'rId13', + relationshipTarget: 'word/header3.xml', + }); + mockApplySectPrToProjection.mockImplementation(() => {}); + + const result = ensureExplicitHeaderFooterSlot(editor as any, { + sectionId: 'section-0', + kind: 'header', + variant: 'default', + addToHistory: false, + }); + + expect(result).not.toBeNull(); + expect(mockApplySectPrToProjection).toHaveBeenCalledWith(editor, expect.anything(), expect.anything(), { + addToHistory: false, + }); + }); + it('clones from inherited source when available', () => { mockSectionProjections.mockReturnValue([createProjection('section-0', 0), createProjection('section-1', 1)]); mockReadTargetSectPr.mockReturnValue(createSectPr()); // no refs on section-1 diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/header-footer-slot-materialization.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/header-footer-slot-materialization.ts index 9ddabfd0b7..f6ab574ef3 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/header-footer-slot-materialization.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/header-footer-slot-materialization.ts @@ -34,6 +34,8 @@ export type EnsureExplicitHeaderFooterSlotInput = { variant: SectionHeaderFooterVariant; /** Optional source part to clone from. If omitted, clones from inherited or creates empty. */ sourceRefId?: string; + /** Whether the sectPr materialization should create an undo step. Defaults to true. */ + addToHistory?: boolean; }; export type EnsureExplicitHeaderFooterSlotResult = { @@ -110,7 +112,7 @@ export function ensureExplicitHeaderFooterSlot( editor: Editor, input: EnsureExplicitHeaderFooterSlotInput, ): EnsureExplicitHeaderFooterSlotResult | null { - const { sectionId, kind, variant, sourceRefId } = input; + const { sectionId, kind, variant, sourceRefId, addToHistory = true } = input; // Step 1–2: Resolve section projections and find the target section. // This is done BEFORE any mutations as a pre-validation gate. @@ -172,7 +174,7 @@ export function ensureExplicitHeaderFooterSlot( // Clone/ensure sectPr and add the new reference const nextSectPr = ensureSectPrElement(currentSectPr); setSectPrHeaderFooterRef(nextSectPr, kind, variant, created.refId); - applySectPrToProjection(editor, projection, nextSectPr); + applySectPrToProjection(editor, projection, nextSectPr, { addToHistory }); } catch { // createHeaderFooterPart committed parts not tracked by this // compoundMutation's snapshot. Clean them up before signalling diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/section-mutation-wrapper.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/section-mutation-wrapper.ts index 98e008285e..4dcd7fc299 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/section-mutation-wrapper.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/section-mutation-wrapper.ts @@ -84,7 +84,14 @@ function getConverter(editor: Editor): ConverterLike | undefined { return (editor as unknown as { converter?: ConverterLike }).converter; } -export function applySectPrToProjection(editor: Editor, projection: SectionProjection, sectPr: XmlElement): void { +export function applySectPrToProjection( + editor: Editor, + projection: SectionProjection, + sectPr: XmlElement, + options?: { addToHistory?: boolean }, +): void { + const addToHistory = options?.addToHistory ?? true; + if (projection.target.kind === 'paragraph') { const paragraph = projection.target.node; const attrs = (paragraph.attrs ?? {}) as Record; @@ -100,6 +107,9 @@ export function applySectPrToProjection(editor: Editor, projection: SectionProje }; const tr = applyDirectMutationMeta(editor.state.tr); + if (!addToHistory) { + tr.setMeta('addToHistory', false); + } tr.setNodeMarkup(projection.target.pos, undefined, nextAttrs, paragraph.marks); tr.setMeta('forceUpdatePagination', true); editor.dispatch(tr); @@ -107,6 +117,9 @@ export function applySectPrToProjection(editor: Editor, projection: SectionProje } const tr = applyDirectMutationMeta(editor.state.tr); + if (!addToHistory) { + tr.setMeta('addToHistory', false); + } tr.setDocAttribute('bodySectPr', sectPr); tr.setMeta('forceUpdatePagination', true); editor.dispatch(tr); diff --git a/packages/super-editor/src/editors/v1/extensions/history/history.js b/packages/super-editor/src/editors/v1/extensions/history/history.js index f960df91cf..156c093904 100644 --- a/packages/super-editor/src/editors/v1/extensions/history/history.js +++ b/packages/super-editor/src/editors/v1/extensions/history/history.js @@ -27,10 +27,13 @@ function applySelectionCleanup(editor, tr) { return cleaned; } -function createHistoryDispatch(editor, dispatch) { +function createHistoryDispatch(editor, dispatch, inputType) { if (!dispatch) return dispatch; return (historyTr) => { - const cleaned = applySelectionCleanup(editor, historyTr); + let cleaned = applySelectionCleanup(editor, historyTr); + if (inputType) { + cleaned = cleaned.setMeta('inputType', inputType); + } dispatch(cleaned); }; } @@ -46,6 +49,69 @@ function runSelectionCleanupAfterCollabHistory(editor) { view.dispatch(tr); } +function getPresentationHistoryProxy(editor) { + const presentationEditor = editor?.presentationEditor ?? editor?._presentationEditor ?? null; + if (!presentationEditor || typeof presentationEditor.getActiveEditor !== 'function') { + return null; + } + + return presentationEditor.getActiveEditor() === editor ? presentationEditor : null; +} + +function resolveHistoryDispatch(editor, allowDispatch) { + if (!allowDispatch) { + return undefined; + } + + return editor?.view?.dispatch?.bind(editor.view) ?? editor?.dispatch?.bind(editor); +} + +export function runEditorUndo(editor, options = {}) { + const state = editor?.state; + const tr = state?.tr; + const allowDispatch = options.allowDispatch !== false; + const inputType = 'historyUndo'; + if (!state || !tr) { + return false; + } + + const dispatch = resolveHistoryDispatch(editor, allowDispatch); + + if (editor.options.collaborationProvider && editor.options.ydoc) { + const result = yUndo(state, dispatch); + if (allowDispatch && result) { + runSelectionCleanupAfterCollabHistory(editor); + } + return result; + } + + const wrappedDispatch = createHistoryDispatch(editor, dispatch, allowDispatch ? inputType : undefined); + return originalUndo(state, wrappedDispatch); +} + +export function runEditorRedo(editor, options = {}) { + const state = editor?.state; + const tr = state?.tr; + const allowDispatch = options.allowDispatch !== false; + const inputType = 'historyRedo'; + if (!state || !tr) { + return false; + } + + const dispatch = resolveHistoryDispatch(editor, allowDispatch); + + if (editor.options.collaborationProvider && editor.options.ydoc) { + const result = yRedo(state, dispatch); + if (allowDispatch && result) { + runSelectionCleanupAfterCollabHistory(editor); + } + return result; + } + + const wrappedDispatch = createHistoryDispatch(editor, dispatch, allowDispatch ? inputType : undefined); + return originalRedo(state, wrappedDispatch); +} + /** * Configuration options for History * @typedef {Object} HistoryOptions @@ -92,16 +158,17 @@ export const History = Extension.create({ * editor.commands.undo() * @note Groups changes within the newGroupDelay window */ - undo: () => ({ state, dispatch, tr }) => { - if (this.editor.options.collaborationProvider && this.editor.options.ydoc) { - tr.setMeta('preventDispatch', true); - const result = yUndo(state); - runSelectionCleanupAfterCollabHistory(this.editor); - return result; + undo: () => ({ tr, dispatch }) => { + tr?.setMeta('preventDispatch', true); + const allowDispatch = typeof dispatch === 'function'; + const presentationEditor = getPresentationHistoryProxy(this.editor); + if (presentationEditor) { + if (allowDispatch && typeof presentationEditor.undo === 'function') { + return Boolean(presentationEditor.undo()); + } + return typeof presentationEditor.canUndo === 'function' ? Boolean(presentationEditor.canUndo()) : false; } - tr.setMeta('inputType', 'historyUndo'); - const wrappedDispatch = createHistoryDispatch(this.editor, dispatch); - return originalUndo(state, wrappedDispatch); + return runEditorUndo(this.editor, { allowDispatch }); }, /** @@ -111,16 +178,17 @@ export const History = Extension.create({ * editor.commands.redo() * @note Only available after an undo action */ - redo: () => ({ state, dispatch, tr }) => { - if (this.editor.options.collaborationProvider && this.editor.options.ydoc) { - tr.setMeta('preventDispatch', true); - const result = yRedo(state); - runSelectionCleanupAfterCollabHistory(this.editor); - return result; + redo: () => ({ tr, dispatch }) => { + tr?.setMeta('preventDispatch', true); + const allowDispatch = typeof dispatch === 'function'; + const presentationEditor = getPresentationHistoryProxy(this.editor); + if (presentationEditor) { + if (allowDispatch && typeof presentationEditor.redo === 'function') { + return Boolean(presentationEditor.redo()); + } + return typeof presentationEditor.canRedo === 'function' ? Boolean(presentationEditor.canRedo()) : false; } - tr.setMeta('inputType', 'historyRedo'); - const wrappedDispatch = createHistoryDispatch(this.editor, dispatch); - return originalRedo(state, wrappedDispatch); + return runEditorRedo(this.editor, { allowDispatch }); }, }; }, diff --git a/packages/super-editor/src/editors/v1/extensions/history/history.test.js b/packages/super-editor/src/editors/v1/extensions/history/history.test.js new file mode 100644 index 0000000000..0b4b3e01e3 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/history/history.test.js @@ -0,0 +1,209 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockOriginalUndo, mockOriginalRedo, mockYUndo, mockYRedo, mockYUndoPlugin } = vi.hoisted(() => ({ + mockOriginalUndo: vi.fn(), + mockOriginalRedo: vi.fn(), + mockYUndo: vi.fn(), + mockYRedo: vi.fn(), + mockYUndoPlugin: vi.fn(() => ({ key: 'mock-y-undo-plugin' })), +})); + +vi.mock('prosemirror-history', () => ({ + history: vi.fn(() => ({ key: 'mock-history-plugin' })), + undo: mockOriginalUndo, + redo: mockOriginalRedo, +})); + +vi.mock('y-prosemirror', () => ({ + undo: mockYUndo, + redo: mockYRedo, + yUndoPlugin: mockYUndoPlugin, +})); + +import { History, runEditorRedo, runEditorUndo } from './history.js'; + +function createEditor(overrides = {}) { + const tr = { + setMeta: vi.fn().mockReturnThis(), + selection: { empty: true }, + setSelection: vi.fn().mockReturnThis(), + }; + const dispatch = vi.fn(); + const editor = { + options: {}, + state: { tr }, + view: { dispatch }, + dispatch, + setOptions: vi.fn(), + ...overrides, + }; + + return { editor, tr, dispatch }; +} + +describe('History extension', () => { + beforeEach(() => { + mockOriginalUndo.mockReset(); + mockOriginalRedo.mockReset(); + mockYUndo.mockReset(); + mockYRedo.mockReset(); + mockYUndoPlugin.mockClear(); + }); + + it.each([ + ['undo', runEditorUndo, mockOriginalUndo, 'historyUndo'], + ['redo', runEditorRedo, mockOriginalRedo, 'historyRedo'], + ])('runs local %s history against the target editor directly', (_label, runner, originalCommand, inputType) => { + const { editor, tr, dispatch } = createEditor(); + const historyTr = { + setMeta: vi.fn().mockReturnThis(), + selection: { empty: true }, + }; + + originalCommand.mockImplementation((_state, wrappedDispatch) => { + wrappedDispatch(historyTr); + return true; + }); + + const result = runner(editor); + + expect(result).toBe(true); + expect(historyTr.setMeta).toHaveBeenCalledWith('inputType', inputType); + expect(tr.setMeta).not.toHaveBeenCalledWith('inputType', inputType); + expect(editor.setOptions).toHaveBeenCalledWith({ + preservedSelection: null, + lastSelection: null, + }); + expect(dispatch).toHaveBeenCalledWith(historyTr); + }); + + it.each([ + ['undo', runEditorUndo, mockOriginalUndo], + ['redo', runEditorRedo, mockOriginalRedo], + ])('keeps local %s side-effect free when dispatch is disallowed', (_label, runner, originalCommand) => { + const { editor, tr, dispatch } = createEditor(); + + originalCommand.mockReturnValue(true); + + const result = runner(editor, { allowDispatch: false }); + + expect(result).toBe(true); + expect(originalCommand).toHaveBeenCalledWith(editor.state, undefined); + expect(tr.setMeta).not.toHaveBeenCalledWith('inputType', expect.anything()); + expect(dispatch).not.toHaveBeenCalled(); + }); + + it.each([ + ['undo', 'undo', mockOriginalUndo], + ['redo', 'redo', mockOriginalRedo], + ])('keeps local %s side-effect free inside can()', (_label, commandName, originalCommand) => { + const { editor, tr, dispatch } = createEditor(); + + originalCommand.mockReturnValue(true); + + const commands = History.config.addCommands.call({ + editor, + options: { depth: 100, newGroupDelay: 500 }, + }); + + const result = commands[commandName]()({ tr, dispatch: undefined }); + + expect(result).toBe(true); + expect(tr.setMeta).toHaveBeenCalledWith('preventDispatch', true); + expect(originalCommand).toHaveBeenCalledWith(editor.state, undefined); + expect(dispatch).not.toHaveBeenCalled(); + }); + + it.each([ + ['undo', runEditorUndo, mockYUndo], + ['redo', runEditorRedo, mockYRedo], + ])('keeps collaborative %s side-effect free when dispatch is disallowed', (_label, runner, collabCommand) => { + const { editor, dispatch } = createEditor({ + options: { collaborationProvider: { id: 'provider' }, ydoc: { guid: 'ydoc' } }, + }); + + collabCommand.mockReturnValue(true); + + const result = runner(editor, { allowDispatch: false }); + + expect(result).toBe(true); + expect(collabCommand).toHaveBeenCalledWith(editor.state, undefined); + expect(dispatch).not.toHaveBeenCalled(); + expect(editor.setOptions).not.toHaveBeenCalled(); + }); + + it.each([ + ['undo', 'undo'], + ['redo', 'redo'], + ])('routes %s through PresentationEditor when the body editor is active', (_label, commandName) => { + const presentationEditor = { + getActiveEditor: vi.fn(), + undo: vi.fn(() => true), + redo: vi.fn(() => true), + canUndo: vi.fn(() => true), + canRedo: vi.fn(() => true), + }; + const { editor, tr } = createEditor({ presentationEditor }); + presentationEditor.getActiveEditor.mockReturnValue(editor); + + const commands = History.config.addCommands.call({ + editor, + options: { depth: 100, newGroupDelay: 500 }, + }); + + const result = commands[commandName]()({ tr, dispatch: () => undefined }); + + expect(result).toBe(true); + expect(tr.setMeta).toHaveBeenCalledWith('preventDispatch', true); + expect(presentationEditor[commandName]).toHaveBeenCalledTimes(1); + }); + + it.each([ + ['undo', 'undo', 'canUndo'], + ['redo', 'redo', 'canRedo'], + ])('keeps presentation-routed %s side-effect free inside can()', (_label, commandName, canMethodName) => { + const presentationEditor = { + getActiveEditor: vi.fn(), + undo: vi.fn(() => true), + redo: vi.fn(() => true), + canUndo: vi.fn(() => true), + canRedo: vi.fn(() => true), + }; + const { editor, tr, dispatch } = createEditor({ presentationEditor }); + presentationEditor.getActiveEditor.mockReturnValue(editor); + + const commands = History.config.addCommands.call({ + editor, + options: { depth: 100, newGroupDelay: 500 }, + }); + + const result = commands[commandName]()({ tr, dispatch: undefined }); + + expect(result).toBe(true); + expect(tr.setMeta).toHaveBeenCalledWith('preventDispatch', true); + expect(presentationEditor[canMethodName]).toHaveBeenCalledTimes(1); + expect(presentationEditor[commandName]).not.toHaveBeenCalled(); + expect(dispatch).not.toHaveBeenCalled(); + }); + + it('keeps body-local undo local when a different presentation surface is active', () => { + const presentationEditor = { + getActiveEditor: vi.fn(() => ({ id: 'footer-editor' })), + undo: vi.fn(() => true), + }; + const { editor, tr } = createEditor({ presentationEditor }); + + mockOriginalUndo.mockReturnValue(true); + + const commands = History.config.addCommands.call({ + editor, + options: { depth: 100, newGroupDelay: 500 }, + }); + + const result = commands.undo()({ tr, dispatch: () => undefined }); + + expect(result).toBe(true); + expect(presentationEditor.undo).not.toHaveBeenCalled(); + expect(mockOriginalUndo).toHaveBeenCalledWith(editor.state, expect.any(Function)); + }); +}); diff --git a/tests/behavior/tests/comments/header-footer-undo-cross-container.spec.ts b/tests/behavior/tests/comments/header-footer-undo-cross-container.spec.ts new file mode 100644 index 0000000000..efecd886ff --- /dev/null +++ b/tests/behavior/tests/comments/header-footer-undo-cross-container.spec.ts @@ -0,0 +1,149 @@ +import type { Page } from '@playwright/test'; +import { expect, test, type SuperDocFixture } from '../../fixtures/superdoc.js'; +import { LONGER_HEADER_SIGN_AREA_DOC_PATH as HEADER_FOOTER_DOC_PATH } from '../../helpers/story-fixtures.js'; +import { + activateFooter, + activateHeader, + getFooterSurfaceLocator, + getHeaderSurfaceLocator, + moveActiveStoryCursorToEnd, + waitForActiveStory, +} from '../../helpers/story-surfaces.js'; +import { assertDocumentApiReady } from '../../helpers/document-api.js'; + +test.use({ + config: { + comments: 'panel', + trackChanges: true, + documentMode: 'suggesting', + showCaret: true, + showSelection: true, + }, +}); + +type SurfaceKind = 'header' | 'footer'; + +async function getHeaderFooterTrackedChangeCount(page: Page, text: string) { + return page.evaluate((insertedText) => { + const comments = (window as any).behaviorHarness?.getCommentsSnapshot?.() ?? []; + return comments.filter( + (comment: any) => + comment?.trackedChange === true && + comment?.trackedChangeText === insertedText && + comment?.trackedChangeStory?.storyType === 'headerFooterPart', + ).length; + }, text); +} + +async function getHeaderFooterSidebarCount(page: Page, text: string) { + return page.evaluate((insertedText) => { + const items = Array.from(document.querySelectorAll('#comments-panel .tracked-change-text')); + return items.filter((item) => (item.textContent ?? '').includes(insertedText)).length; + }, text); +} + +async function activateSurface(superdoc: SuperDocFixture, surface: SurfaceKind) { + if (surface === 'header') { + return activateHeader(superdoc); + } + return activateFooter(superdoc); +} + +function getSurfaceLocator(page: Page, surface: SurfaceKind) { + return surface === 'header' ? getHeaderSurfaceLocator(page) : getFooterSurfaceLocator(page); +} + +async function clickBodySurface(page: Page) { + const bodyLine = page.locator('.superdoc-line').first(); + await bodyLine.scrollIntoViewIfNeeded(); + await bodyLine.click(); +} + +async function activateBlankDocumentHeader(superdoc: SuperDocFixture) { + const pageSurface = superdoc.page.locator('.superdoc-page').first(); + await pageSurface.waitFor({ state: 'visible', timeout: 15_000 }); + const box = await pageSurface.boundingBox(); + expect(box).toBeTruthy(); + + await superdoc.page.mouse.dblclick(box!.x + 120, box!.y + 60); + await superdoc.waitForStable(); + await waitForActiveStory(superdoc.page, { storyType: 'headerFooterPart' }); + + return getHeaderSurfaceLocator(superdoc.page); +} + +async function clickBlankDocumentBody(page: Page) { + const pageSurface = page.locator('.superdoc-page').first(); + const box = await pageSurface.boundingBox(); + expect(box).toBeTruthy(); + await page.mouse.click(box!.x + 140, box!.y + 180); +} + +for (const surface of ['header', 'footer'] as const) { + test(`undo/redo from the body restores tracked ${surface} edits after leaving the active story`, async ({ + superdoc, + }) => { + const insertedText = surface === 'header' ? 'HDRUNDO' : 'FTRUNDO'; + + await assertDocumentApiReady(superdoc.page); + await superdoc.loadDocument(HEADER_FOOTER_DOC_PATH); + await superdoc.waitForStable(); + + const surfaceLocator = getSurfaceLocator(superdoc.page, surface); + await activateSurface(superdoc, surface); + await moveActiveStoryCursorToEnd(superdoc.page); + await superdoc.page.keyboard.insertText(insertedText); + await superdoc.waitForStable(); + + await expect(surfaceLocator).toContainText(insertedText); + await expect.poll(() => getHeaderFooterTrackedChangeCount(superdoc.page, insertedText)).toBe(1); + await expect.poll(() => getHeaderFooterSidebarCount(superdoc.page, insertedText)).toBe(1); + + await clickBodySurface(superdoc.page); + await superdoc.waitForStable(); + await waitForActiveStory(superdoc.page, null); + + await superdoc.undo(); + await superdoc.waitForStable(); + + await expect(surfaceLocator).not.toContainText(insertedText); + await expect.poll(() => getHeaderFooterTrackedChangeCount(superdoc.page, insertedText)).toBe(0); + await expect.poll(() => getHeaderFooterSidebarCount(superdoc.page, insertedText)).toBe(0); + + await superdoc.redo(); + await superdoc.waitForStable(); + + await expect(surfaceLocator).toContainText(insertedText); + await expect.poll(() => getHeaderFooterTrackedChangeCount(superdoc.page, insertedText)).toBe(1); + await expect.poll(() => getHeaderFooterSidebarCount(superdoc.page, insertedText)).toBe(1); + }); +} + +test('undo from the body removes blank-document tracked header edits after leaving the active story', async ({ + superdoc, +}) => { + const insertedText = 'BLANKHDRUNDO'; + + await assertDocumentApiReady(superdoc.page); + await superdoc.waitForStable(); + + const headerSurface = await activateBlankDocumentHeader(superdoc); + await moveActiveStoryCursorToEnd(superdoc.page); + await superdoc.page.keyboard.insertText(insertedText); + await superdoc.waitForStable(); + + await expect(headerSurface).toContainText(insertedText); + await expect.poll(() => getHeaderFooterTrackedChangeCount(superdoc.page, insertedText)).toBe(1); + await expect.poll(() => getHeaderFooterSidebarCount(superdoc.page, insertedText)).toBe(1); + + await clickBlankDocumentBody(superdoc.page); + await superdoc.waitForStable(); + await waitForActiveStory(superdoc.page, null); + + await superdoc.undo(); + await superdoc.waitForStable(); + + await expect(headerSurface).not.toContainText(insertedText); + await expect.poll(() => getHeaderFooterTrackedChangeCount(superdoc.page, insertedText)).toBe(0); + await expect.poll(() => getHeaderFooterSidebarCount(superdoc.page, insertedText)).toBe(0); +});