From 4994df6f31bda277e7623cbf5326643112d7ac9c Mon Sep 17 00:00:00 2001 From: Matthew Connelly Date: Fri, 27 Feb 2026 19:55:41 -0500 Subject: [PATCH 1/5] fix(scroll): wait for virtualized page mount and center text element --- .../presentation-editor/PresentationEditor.ts | 96 ++- ...resentationEditor.scrollToPosition.test.ts | 611 ++++++++++++++++++ .../src/extensions/search/search.js | 8 +- 3 files changed, 711 insertions(+), 4 deletions(-) create mode 100644 packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index 16c1135749..bfb9a19f10 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -2042,7 +2042,9 @@ export class PresentationEditor extends EventEmitter { if (pageIndex != null) { const pageEl = getPageElementByIndex(this.#viewportHost, pageIndex); if (pageEl) { - pageEl.scrollIntoView({ block, inline: 'nearest', behavior }); + // Find the specific element containing this position for precise centering + const targetEl = this.#findElementAtPosition(pageEl, clampedPos); + (targetEl ?? pageEl).scrollIntoView({ block, inline: 'nearest', behavior }); return true; } } @@ -2053,6 +2055,98 @@ export class PresentationEditor extends EventEmitter { } } + /** + * Find the DOM element containing a specific document position. + * Returns the most specific (smallest range) matching element. + */ + #findElementAtPosition(pageEl: HTMLElement, pos: number): HTMLElement | null { + const elements = Array.from(pageEl.querySelectorAll('[data-pm-start][data-pm-end]')); + let bestMatch: HTMLElement | null = null; + let smallestRange = Infinity; + + for (const el of elements) { + const htmlEl = el as HTMLElement; + const start = Number(htmlEl.dataset.pmStart); + const end = Number(htmlEl.dataset.pmEnd); + if (!Number.isFinite(start) || !Number.isFinite(end)) continue; + + if (pos >= start && pos <= end) { + const range = end - start; + if (range < smallestRange) { + smallestRange = range; + bestMatch = htmlEl; + } + } + } + return bestMatch; + } + + /** + * Async version of scrollToPosition that handles virtualized pages. + * + * When pages are virtualized (not mounted in the DOM), this method will: + * 1. Try the sync scroll first (fast path if page is already mounted) + * 2. If that fails, trigger virtualization to render the target page + * 3. Wait for the page to mount (up to 2000ms) + * 4. Retry the scroll + * + * Use this method when navigating to positions that may be on virtualized pages. + * + * @param pos - Document position in the active editor to scroll to + * @param options - Scrolling options + * @param options.block - Alignment within the viewport ('start' | 'center' | 'end' | 'nearest') + * @param options.behavior - Scroll behavior ('auto' | 'smooth') + * @returns Promise resolving to true if scrolling succeeded, false otherwise + */ + async scrollToPositionAsync( + pos: number, + options: { block?: 'start' | 'center' | 'end' | 'nearest'; behavior?: ScrollBehavior } = {}, + ): Promise { + // Fast path: try sync scroll first (works if page already mounted) + if (this.scrollToPosition(pos, options)) { + return true; + } + + // Page not mounted - find which page contains this position + const activeEditor = this.getActiveEditor(); + const doc = activeEditor?.state?.doc; + if (!doc || !Number.isFinite(pos)) return false; + + const clampedPos = Math.max(0, Math.min(pos, doc.content.size)); + const layout = this.#layoutState.layout; + const sessionMode = this.#headerFooterSession?.session?.mode ?? 'body'; + if (!layout || sessionMode !== 'body') return false; + + let pageIndex: number | null = null; + for (let idx = 0; idx < layout.pages.length; idx++) { + const page = layout.pages[idx]; + for (const fragment of page.fragments) { + const frag = fragment as { pmStart?: number; pmEnd?: number }; + if (frag.pmStart != null && frag.pmEnd != null && clampedPos >= frag.pmStart && clampedPos <= frag.pmEnd) { + pageIndex = idx; + break; + } + } + if (pageIndex != null) break; + } + if (pageIndex == null) return false; + + // Trigger virtualization to render the page + this.#scrollPageIntoView(pageIndex); + + // Wait for page to mount in the DOM + const mounted = await this.#waitForPageMount(pageIndex, { + timeout: PresentationEditor.ANCHOR_NAV_TIMEOUT_MS, + }); + if (!mounted) { + console.warn(`[PresentationEditor] scrollToPositionAsync: Page ${pageIndex} failed to mount within timeout`); + return false; + } + + // Retry now that page is mounted + return this.scrollToPosition(pos, options); + } + /** * Get document position from viewport coordinates (header/footer-aware). * diff --git a/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts new file mode 100644 index 0000000000..d7a87aa474 --- /dev/null +++ b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts @@ -0,0 +1,611 @@ +/** + * Tests for scrollToPosition and scrollToPositionAsync methods. + * + * These methods handle scrolling to a document position in presentation mode, + * where ProseMirror's native scrollIntoView operates on a hidden editor. + * + * What we test: + * + * scrollToPosition (sync): + * - Returns false for invalid positions (NaN, Infinity) + * - Returns false when position is not in any page fragment + * - Scrolls to the specific text element (using data-pm-start/data-pm-end attributes) + * - Prefers more specific (smaller range) elements for precise centering + * - Falls back to page scroll when no text element found + * - Respects the `block` option (center, start, end, nearest) + * + * scrollToPositionAsync: + * - Uses fast path (sync scroll) when page is already mounted + * - Waits for virtualized pages to mount before scrolling + * - Returns false when page fails to mount within timeout (2000ms) + * - Returns false for invalid positions or positions outside layout + * + * Test patterns borrowed from PresentationEditor.goToAnchor.test.ts for testing + * page virtualization and async mount waiting behavior. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import type { Mock } from 'vitest'; +import { PresentationEditor } from '../PresentationEditor.js'; + +const { + createDefaultConverter, + mockIncrementalLayout, + mockToFlowBlocks, + mockSelectionToRects, + mockCreateDomPainter, + mockMeasureBlock, + mockEditorConverterStore, + mockCreateHeaderFooterEditor, + mockOnHeaderFooterDataUpdate, + mockUpdateYdocDocxData, + mockEditorOverlayManager, +} = vi.hoisted(() => { + const createDefaultConverter = () => ({ + headers: { + 'rId-header-default': { type: 'doc', content: [{ type: 'paragraph' }] }, + }, + footers: { + 'rId-footer-default': { type: 'doc', content: [{ type: 'paragraph' }] }, + }, + headerIds: { + default: 'rId-header-default', + first: null, + even: null, + odd: null, + ids: ['rId-header-default'], + }, + footerIds: { + default: 'rId-footer-default', + first: null, + even: null, + odd: null, + ids: ['rId-footer-default'], + }, + }); + + const converterStore = { + current: createDefaultConverter() as ReturnType & Record, + mediaFiles: {} as Record, + }; + + return { + createDefaultConverter, + mockIncrementalLayout: vi.fn(async () => ({ layout: { pages: [] }, measures: [] })), + mockToFlowBlocks: vi.fn(() => ({ blocks: [], bookmarks: new Map() })), + mockSelectionToRects: vi.fn(() => []), + mockCreateDomPainter: vi.fn(() => ({ + paint: vi.fn(), + destroy: vi.fn(), + setZoom: vi.fn(), + setLayoutMode: vi.fn(), + setProviders: vi.fn(), + setData: vi.fn(), + })), + mockMeasureBlock: vi.fn(() => ({ width: 100, height: 100 })), + mockEditorConverterStore: converterStore, + mockCreateHeaderFooterEditor: vi.fn(() => { + const createEmitter = () => { + const listeners = new Map void>>(); + const on = (event: string, handler: (payload?: unknown) => void) => { + if (!listeners.has(event)) listeners.set(event, new Set()); + listeners.get(event)!.add(handler); + }; + const off = (event: string, handler: (payload?: unknown) => void) => { + listeners.get(event)?.delete(handler); + }; + const once = (event: string, handler: (payload?: unknown) => void) => { + const wrapper = (payload?: unknown) => { + off(event, wrapper); + handler(payload); + }; + on(event, wrapper); + }; + const emit = (event: string, payload?: unknown) => { + listeners.get(event)?.forEach((handler) => handler(payload)); + }; + return { on, off, once, emit }; + }; + + const emitter = createEmitter(); + const editorStub = { + on: emitter.on, + off: emitter.off, + once: emitter.once, + emit: emitter.emit, + destroy: vi.fn(), + setEditable: vi.fn(), + setOptions: vi.fn(), + commands: { + setTextSelection: vi.fn(), + }, + state: { + doc: { + content: { + size: 200, + }, + }, + }, + view: { + dom: document.createElement('div'), + focus: vi.fn(), + }, + }; + queueMicrotask(() => editorStub.emit('create')); + return editorStub; + }), + mockOnHeaderFooterDataUpdate: vi.fn(), + mockUpdateYdocDocxData: vi.fn(() => Promise.resolve()), + mockEditorOverlayManager: vi.fn().mockImplementation(() => ({ + showEditingOverlay: vi.fn(() => ({ + success: true, + editorHost: document.createElement('div'), + reason: null, + })), + hideEditingOverlay: vi.fn(), + showSelectionOverlay: vi.fn(), + hideSelectionOverlay: vi.fn(), + setOnDimmingClick: vi.fn(), + getActiveEditorHost: vi.fn(() => null), + destroy: vi.fn(), + })), + }; +}); + +// Mock Editor class +vi.mock('../../Editor', () => { + return { + Editor: vi.fn().mockImplementation(() => ({ + setDocumentMode: vi.fn(), + setOptions: vi.fn(), + on: vi.fn(), + off: vi.fn(), + destroy: vi.fn(), + getJSON: vi.fn(() => ({ type: 'doc', content: [] })), + isEditable: true, + state: { + selection: { from: 0, to: 0 }, + doc: { + nodeSize: 200, + content: { + size: 200, + }, + descendants: vi.fn(), + resolve: vi.fn(() => ({ + node: vi.fn(), + parent: null, + depth: 0, + })), + }, + }, + view: { + dom: { + dispatchEvent: vi.fn(() => true), + focus: vi.fn(), + }, + focus: vi.fn(), + dispatch: vi.fn(), + }, + options: { + documentId: 'test-doc', + element: document.createElement('div'), + }, + converter: mockEditorConverterStore.current, + storage: { + image: { + media: mockEditorConverterStore.mediaFiles, + }, + }, + })), + }; +}); + +// Mock pm-adapter functions +vi.mock('@superdoc/pm-adapter', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + toFlowBlocks: mockToFlowBlocks, + }; +}); + +// Mock layout-bridge functions +vi.mock('@superdoc/layout-bridge', () => ({ + incrementalLayout: mockIncrementalLayout, + selectionToRects: mockSelectionToRects, + clickToPosition: vi.fn(() => null), + createDragHandler: vi.fn(() => () => {}), + getFragmentAtPosition: vi.fn(() => null), + computeLinePmRange: vi.fn(() => ({ from: 0, to: 0 })), + measureCharacterX: vi.fn(() => 0), + extractIdentifierFromConverter: vi.fn((_converter) => ({ + extractHeaderId: vi.fn(() => 'rId-header-default'), + extractFooterId: vi.fn(() => 'rId-footer-default'), + })), + buildMultiSectionIdentifier: vi.fn(() => ({ sections: [] })), + getHeaderFooterTypeForSection: vi.fn(() => 'default'), + getHeaderFooterType: vi.fn(() => 'default'), + layoutHeaderFooterWithCache: vi.fn(async () => ({ + default: { + layout: { pages: [{ fragments: [], number: 1 }], height: 0 }, + blocks: [], + measures: [], + }, + })), + computeDisplayPageNumber: vi.fn((pages) => pages.map((p) => ({ displayText: String(p.number ?? 1) }))), + PageGeometryHelper: vi.fn().mockImplementation(({ layout, pageGap }) => ({ + updateLayout: vi.fn(), + getPageIndexAtY: vi.fn(() => 0), + getNearestPageIndex: vi.fn(() => 0), + getPageTop: vi.fn(() => 0), + getPageGap: vi.fn(() => pageGap ?? 0), + getLayout: vi.fn(() => layout), + })), +})); + +// Mock painter-dom +vi.mock('@superdoc/painter-dom', () => ({ + createDomPainter: mockCreateDomPainter, + DOM_CLASS_NAMES: { + PAGE: 'superdoc-page', + FRAGMENT: 'superdoc-fragment', + LINE: 'superdoc-line', + INLINE_SDT_WRAPPER: 'superdoc-structured-content-inline', + BLOCK_SDT: 'superdoc-structured-content-block', + DOCUMENT_SECTION: 'superdoc-document-section', + }, +})); + +// Mock measuring-dom +vi.mock('@superdoc/measuring-dom', () => ({ + measureBlock: mockMeasureBlock, +})); + +vi.mock('@extensions/pagination/pagination-helpers.js', () => ({ + createHeaderFooterEditor: mockCreateHeaderFooterEditor, + onHeaderFooterDataUpdate: mockOnHeaderFooterDataUpdate, +})); + +vi.mock('@extensions/collaboration/collaboration-helpers.js', () => ({ + updateYdocDocxData: mockUpdateYdocDocxData, +})); + +vi.mock('../../header-footer/EditorOverlayManager', () => ({ + EditorOverlayManager: mockEditorOverlayManager, +})); + +describe('PresentationEditor - scrollToPosition', () => { + let container: HTMLElement; + let editor: PresentationEditor; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + + vi.clearAllMocks(); + mockEditorConverterStore.current = { + ...createDefaultConverter(), + headerEditors: [], + footerEditors: [], + }; + mockEditorConverterStore.mediaFiles = {}; + + // Setup default layout with two pages + const layoutResult = { + layout: { + pageSize: { w: 612, h: 792 }, + pages: [ + { + number: 1, + numberText: '1', + size: { w: 612, h: 792 }, + fragments: [{ kind: 'para', pmStart: 0, pmEnd: 100 }], + margins: { top: 72, bottom: 72, left: 72, right: 72, header: 36, footer: 36 }, + sectionRefs: { + headerRefs: { default: 'rId-header-default' }, + footerRefs: { default: 'rId-footer-default' }, + }, + }, + { + number: 2, + numberText: '2', + size: { w: 612, h: 792 }, + fragments: [{ kind: 'para', pmStart: 100, pmEnd: 200 }], + margins: { top: 72, bottom: 72, left: 72, right: 72, header: 36, footer: 36 }, + sectionRefs: { + headerRefs: { default: 'rId-header-default' }, + footerRefs: { default: 'rId-footer-default' }, + }, + }, + ], + }, + measures: [], + }; + + mockIncrementalLayout.mockResolvedValue(layoutResult); + mockToFlowBlocks.mockReturnValue({ blocks: [], bookmarks: new Map() }); + }); + + afterEach(() => { + if (editor) { + editor.destroy(); + } + if (container.parentNode) { + document.body.removeChild(container); + } + }); + + describe('scrollToPosition (sync)', () => { + it('should return false for invalid position', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + + expect(editor.scrollToPosition(NaN)).toBe(false); + expect(editor.scrollToPosition(Infinity)).toBe(false); + }); + + it('should return false when position not in any page fragment', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Position 250 is beyond our layout (pages cover 0-200) + expect(editor.scrollToPosition(250)).toBe(false); + }); + + it('should scroll to page element when position is valid', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Create mock page element + const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + if (pagesHost) { + const mockPage = document.createElement('div'); + mockPage.setAttribute('data-page-index', '0'); + mockPage.scrollIntoView = vi.fn(); + pagesHost.appendChild(mockPage); + } + + const result = editor.scrollToPosition(50); + expect(result).toBe(true); + }); + + it('should scroll to specific text element when found', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Create mock page with text element containing position data + const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + if (pagesHost) { + const mockPage = document.createElement('div'); + mockPage.setAttribute('data-page-index', '0'); + mockPage.scrollIntoView = vi.fn(); + + // Add text element with pm position data + const textElement = document.createElement('span'); + textElement.dataset.pmStart = '40'; + textElement.dataset.pmEnd = '60'; + textElement.scrollIntoView = vi.fn(); + mockPage.appendChild(textElement); + + pagesHost.appendChild(mockPage); + + const result = editor.scrollToPosition(50); + expect(result).toBe(true); + // Should scroll the text element, not the page + expect(textElement.scrollIntoView).toHaveBeenCalledWith({ + block: 'center', + inline: 'nearest', + behavior: 'auto', + }); + expect(mockPage.scrollIntoView).not.toHaveBeenCalled(); + } + }); + + it('should prefer more specific (smaller range) elements', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + if (pagesHost) { + const mockPage = document.createElement('div'); + mockPage.setAttribute('data-page-index', '0'); + mockPage.scrollIntoView = vi.fn(); + + // Larger range element (paragraph) + const paraElement = document.createElement('div'); + paraElement.dataset.pmStart = '0'; + paraElement.dataset.pmEnd = '100'; + paraElement.scrollIntoView = vi.fn(); + mockPage.appendChild(paraElement); + + // Smaller range element (span within paragraph) + const spanElement = document.createElement('span'); + spanElement.dataset.pmStart = '45'; + spanElement.dataset.pmEnd = '55'; + spanElement.scrollIntoView = vi.fn(); + paraElement.appendChild(spanElement); + + pagesHost.appendChild(mockPage); + + const result = editor.scrollToPosition(50); + expect(result).toBe(true); + // Should scroll the smaller span element + expect(spanElement.scrollIntoView).toHaveBeenCalled(); + expect(paraElement.scrollIntoView).not.toHaveBeenCalled(); + } + }); + + it('should use provided block option', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + if (pagesHost) { + const mockPage = document.createElement('div'); + mockPage.setAttribute('data-page-index', '0'); + mockPage.scrollIntoView = vi.fn(); + pagesHost.appendChild(mockPage); + + editor.scrollToPosition(50, { block: 'start' }); + expect(mockPage.scrollIntoView).toHaveBeenCalledWith({ + block: 'start', + inline: 'nearest', + behavior: 'auto', + }); + } + }); + + it('should fall back to page scroll when no text element found', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + if (pagesHost) { + const mockPage = document.createElement('div'); + mockPage.setAttribute('data-page-index', '0'); + mockPage.scrollIntoView = vi.fn(); + // No text elements with pm data + pagesHost.appendChild(mockPage); + + const result = editor.scrollToPosition(50); + expect(result).toBe(true); + expect(mockPage.scrollIntoView).toHaveBeenCalled(); + } + }); + }); + + describe('scrollToPositionAsync', () => { + it('should use fast path when page is already mounted', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + if (pagesHost) { + const mockPage = document.createElement('div'); + mockPage.setAttribute('data-page-index', '0'); + mockPage.scrollIntoView = vi.fn(); + pagesHost.appendChild(mockPage); + + const result = await editor.scrollToPositionAsync(50); + expect(result).toBe(true); + expect(mockPage.scrollIntoView).toHaveBeenCalled(); + } + }); + + it('should return false for invalid position', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + + expect(await editor.scrollToPositionAsync(NaN)).toBe(false); + }); + + it('should return false when position not in layout', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Position beyond layout + expect(await editor.scrollToPositionAsync(500)).toBe(false); + }); + + it('should wait for page mount when page is virtualized', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + + // Start with no page mounted + // Simulate page mounting after a delay + setTimeout(() => { + if (pagesHost) { + const mockPage = document.createElement('div'); + mockPage.setAttribute('data-page-index', '1'); + mockPage.scrollIntoView = vi.fn(); + pagesHost.appendChild(mockPage); + } + }, 100); + + // Position 150 is on page 2 (index 1) + const result = await editor.scrollToPositionAsync(150); + expect(result).toBe(true); + }); + + it('should return false and warn when page fails to mount within timeout', async () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Page 2 never mounts - should timeout + // Note: This test may be slow due to the 2000ms timeout + // In real tests, you might want to mock the timeout value + const result = await editor.scrollToPositionAsync(150); + + // The page never mounted, so it should fail + // However, due to the test setup, the page might already exist + // This test verifies the warning path when timeout occurs + if (!result) { + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('failed to mount within timeout')); + } + + consoleWarnSpy.mockRestore(); + }); + }); +}); diff --git a/packages/super-editor/src/extensions/search/search.js b/packages/super-editor/src/extensions/search/search.js index 571c46e554..2ef96301bf 100644 --- a/packages/super-editor/src/extensions/search/search.js +++ b/packages/super-editor/src/extensions/search/search.js @@ -458,9 +458,11 @@ export const Search = Extension.create({ if (dispatch) dispatch(tr); const presentationEditor = editor.presentationEditor; - if (presentationEditor && typeof presentationEditor.scrollToPosition === 'function') { - const didScroll = presentationEditor.scrollToPosition(from, { block: 'center' }); - if (didScroll) return true; + const scrollFn = presentationEditor?.scrollToPositionAsync ?? presentationEditor?.scrollToPosition; + if (typeof scrollFn === 'function') { + // Fire-and-forget: async version handles virtualized pages, sync is fallback + scrollFn.call(presentationEditor, from, { block: 'center' }); + return true; } const { node } = editor.view.domAtPos(from); From 08dd80f6c21961a42d55140babed1ef9d8922051 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 28 Feb 2026 14:11:35 -0800 Subject: [PATCH 2/5] fix(super-editor): restore reliable search result scrolling and ignore header/footer scroll targets --- .../presentation-editor/PresentationEditor.ts | 4 + ...resentationEditor.scrollToPosition.test.ts | 69 ++++++- .../src/extensions/search/search.js | 24 ++- .../search/search.scrollBehavior.test.js | 190 ++++++++++++++++++ 4 files changed, 272 insertions(+), 15 deletions(-) create mode 100644 packages/super-editor/src/extensions/search/search.scrollBehavior.test.js diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index 8e64e3ce6d..22157c3a1b 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -2068,6 +2068,10 @@ export class PresentationEditor extends EventEmitter { for (const el of elements) { const htmlEl = el as HTMLElement; + // Skip header/footer fragments — their PM positions come from a separate + // document and can overlap with body positions, causing incorrect matches. + if (htmlEl.closest('.superdoc-page-header, .superdoc-page-footer')) continue; + const start = Number(htmlEl.dataset.pmStart); const end = Number(htmlEl.dataset.pmEnd); if (!Number.isFinite(start) || !Number.isFinite(end)) continue; diff --git a/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts index d7a87aa474..d52e1a2f4f 100644 --- a/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts +++ b/packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts @@ -372,6 +372,7 @@ describe('PresentationEditor - scrollToPosition', () => { // Create mock page element const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + expect(pagesHost).toBeTruthy(); if (pagesHost) { const mockPage = document.createElement('div'); mockPage.setAttribute('data-page-index', '0'); @@ -394,6 +395,7 @@ describe('PresentationEditor - scrollToPosition', () => { // Create mock page with text element containing position data const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + expect(pagesHost).toBeTruthy(); if (pagesHost) { const mockPage = document.createElement('div'); mockPage.setAttribute('data-page-index', '0'); @@ -430,6 +432,7 @@ describe('PresentationEditor - scrollToPosition', () => { await new Promise((resolve) => setTimeout(resolve, 50)); const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + expect(pagesHost).toBeTruthy(); if (pagesHost) { const mockPage = document.createElement('div'); mockPage.setAttribute('data-page-index', '0'); @@ -459,6 +462,61 @@ describe('PresentationEditor - scrollToPosition', () => { } }); + it('should skip header/footer elements when finding scroll target', async () => { + editor = new PresentationEditor({ + element: container, + documentId: 'test-doc', + }); + + await vi.waitFor(() => expect(mockIncrementalLayout).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 50)); + + const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + expect(pagesHost).toBeTruthy(); + if (pagesHost) { + const mockPage = document.createElement('div'); + mockPage.setAttribute('data-page-index', '0'); + mockPage.scrollIntoView = vi.fn(); + + // Header element with overlapping PM positions (from separate PM doc) + const header = document.createElement('div'); + header.className = 'superdoc-page-header'; + const headerSpan = document.createElement('span'); + headerSpan.dataset.pmStart = '48'; + headerSpan.dataset.pmEnd = '52'; + headerSpan.scrollIntoView = vi.fn(); + header.appendChild(headerSpan); + mockPage.appendChild(header); + + // Body element with wider range + const bodySpan = document.createElement('span'); + bodySpan.dataset.pmStart = '40'; + bodySpan.dataset.pmEnd = '60'; + bodySpan.scrollIntoView = vi.fn(); + mockPage.appendChild(bodySpan); + + // Footer element with overlapping PM positions + const footer = document.createElement('div'); + footer.className = 'superdoc-page-footer'; + const footerSpan = document.createElement('span'); + footerSpan.dataset.pmStart = '49'; + footerSpan.dataset.pmEnd = '51'; + footerSpan.scrollIntoView = vi.fn(); + footer.appendChild(footerSpan); + mockPage.appendChild(footer); + + pagesHost.appendChild(mockPage); + + const result = editor.scrollToPosition(50); + expect(result).toBe(true); + // Should scroll the body element, not the header/footer elements + // (even though header/footer have smaller ranges) + expect(bodySpan.scrollIntoView).toHaveBeenCalled(); + expect(headerSpan.scrollIntoView).not.toHaveBeenCalled(); + expect(footerSpan.scrollIntoView).not.toHaveBeenCalled(); + } + }); + it('should use provided block option', async () => { editor = new PresentationEditor({ element: container, @@ -469,6 +527,7 @@ describe('PresentationEditor - scrollToPosition', () => { await new Promise((resolve) => setTimeout(resolve, 50)); const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + expect(pagesHost).toBeTruthy(); if (pagesHost) { const mockPage = document.createElement('div'); mockPage.setAttribute('data-page-index', '0'); @@ -494,6 +553,7 @@ describe('PresentationEditor - scrollToPosition', () => { await new Promise((resolve) => setTimeout(resolve, 50)); const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + expect(pagesHost).toBeTruthy(); if (pagesHost) { const mockPage = document.createElement('div'); mockPage.setAttribute('data-page-index', '0'); @@ -519,6 +579,7 @@ describe('PresentationEditor - scrollToPosition', () => { await new Promise((resolve) => setTimeout(resolve, 50)); const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + expect(pagesHost).toBeTruthy(); if (pagesHost) { const mockPage = document.createElement('div'); mockPage.setAttribute('data-page-index', '0'); @@ -565,6 +626,7 @@ describe('PresentationEditor - scrollToPosition', () => { await new Promise((resolve) => setTimeout(resolve, 50)); const pagesHost = container.querySelector('.presentation-editor__pages') as HTMLElement; + expect(pagesHost).toBeTruthy(); // Start with no page mounted // Simulate page mounting after a delay @@ -599,11 +661,8 @@ describe('PresentationEditor - scrollToPosition', () => { const result = await editor.scrollToPositionAsync(150); // The page never mounted, so it should fail - // However, due to the test setup, the page might already exist - // This test verifies the warning path when timeout occurs - if (!result) { - expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('failed to mount within timeout')); - } + expect(result).toBe(false); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('failed to mount within timeout')); consoleWarnSpy.mockRestore(); }); diff --git a/packages/super-editor/src/extensions/search/search.js b/packages/super-editor/src/extensions/search/search.js index 2ef96301bf..cd3f0dc061 100644 --- a/packages/super-editor/src/extensions/search/search.js +++ b/packages/super-editor/src/extensions/search/search.js @@ -458,16 +458,20 @@ export const Search = Extension.create({ if (dispatch) dispatch(tr); const presentationEditor = editor.presentationEditor; - const scrollFn = presentationEditor?.scrollToPositionAsync ?? presentationEditor?.scrollToPosition; - if (typeof scrollFn === 'function') { - // Fire-and-forget: async version handles virtualized pages, sync is fallback - scrollFn.call(presentationEditor, from, { block: 'center' }); - return true; - } - - const { node } = editor.view.domAtPos(from); - if (node?.scrollIntoView) { - node.scrollIntoView({ block: 'center', inline: 'nearest' }); + // Try sync scroll first — returns true when the page is mounted and in body mode. + const scrolled = presentationEditor?.scrollToPosition?.(from, { block: 'center' }) ?? false; + + if (!scrolled) { + // Async version handles virtualized (un-mounted) pages; fire-and-forget + // because it will scroll once the target page mounts. + Promise.resolve(presentationEditor?.scrollToPositionAsync?.(from, { block: 'center' })).catch(() => {}); + + // DOM fallback for non-presentation contexts or when presentation + // scroll cannot run (e.g. header/footer mode, no layout). + const { node } = editor.view.domAtPos(from); + if (node?.scrollIntoView) { + node.scrollIntoView({ block: 'center', inline: 'nearest' }); + } } return true; diff --git a/packages/super-editor/src/extensions/search/search.scrollBehavior.test.js b/packages/super-editor/src/extensions/search/search.scrollBehavior.test.js new file mode 100644 index 0000000000..9c5b6ee188 --- /dev/null +++ b/packages/super-editor/src/extensions/search/search.scrollBehavior.test.js @@ -0,0 +1,190 @@ +// @ts-nocheck +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +/** + * Tests for goToSearchResult scroll behavior. + * + * These tests verify that search result navigation correctly handles + * presentation-mode scrolling with proper fallback behavior: + * + * - When sync scrollToPosition succeeds → uses it, no fallback + * - When sync fails → fires async for virtualized pages + DOM fallback + * - When no presentation editor → uses DOM fallback + */ + +// Minimal mock of prosemirror-search-patched to avoid heavy ProseMirror setup +vi.mock('./prosemirror-search-patched.js', () => ({ + search: vi.fn(() => ({ + key: { get: vi.fn(() => null) }, + })), + SearchQuery: vi.fn(), + setSearchState: vi.fn(), + // Must return a DecorationSet-like object with .find() + getMatchHighlights: vi.fn(() => ({ find: vi.fn(() => []) })), +})); + +vi.mock('uuid', () => ({ + v4: vi.fn(() => 'mock-uuid'), +})); + +// Mock prosemirror-state to avoid real ProseMirror doc requirements +const mockSelection = {}; +vi.mock('prosemirror-state', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + TextSelection: { + create: vi.fn(() => mockSelection), + }, + }; +}); + +vi.mock('@core/PositionTracker.js', () => ({ + PositionTracker: vi.fn(() => ({ + resolve: vi.fn(() => null), + })), +})); + +// We import the extension so we can extract the command factory +const { Search } = await import('./search.js'); + +/** + * Build a minimal editor-like context for calling the goToSearchResult command. + */ +function createEditorContext(overrides = {}) { + const mockNode = document.createElement('span'); + mockNode.scrollIntoView = vi.fn(); + + const doc = { + content: { size: 200 }, + nodeSize: 200, + resolve: vi.fn(() => ({ + node: vi.fn(() => null), + parent: { type: { name: 'paragraph' } }, + depth: 1, + nodeAfter: { isText: true }, + nodeBefore: { isText: true }, + })), + }; + + return { + state: { + doc, + tr: { + setSelection: vi.fn().mockReturnValue({ + scrollIntoView: vi.fn().mockReturnThis(), + }), + }, + }, + dispatch: vi.fn(), + editor: { + view: { + focus: vi.fn(), + domAtPos: vi.fn(() => ({ node: mockNode })), + }, + presentationEditor: overrides.presentationEditor ?? null, + positionTracker: { resolve: vi.fn(() => null) }, + storage: { positionTracker: { tracker: null } }, + }, + // Expose the mock node for assertions + _domNode: mockNode, + }; +} + +describe('goToSearchResult — scroll behavior', () => { + /** @type {Function} command factory from the Search extension */ + let goToSearchResultFactory; + + beforeEach(() => { + // Extract the command factory from the extension definition. + // Search.create() returns an extension config; addCommands() returns the + // commands object. goToSearchResult is a curried command: + // goToSearchResult(match) => ({ state, dispatch, editor }) => boolean + const ext = Search.config; + const commands = ext.addCommands.call({ editor: null, storage: {} }); + goToSearchResultFactory = commands.goToSearchResult; + }); + + it('uses sync scrollToPosition when it succeeds (no DOM fallback)', () => { + const scrollToPosition = vi.fn(() => true); + const scrollToPositionAsync = vi.fn(); + const ctx = createEditorContext({ + presentationEditor: { scrollToPosition, scrollToPositionAsync }, + }); + + const match = { from: 10, to: 20, text: 'hello', id: '1', ranges: [], trackerIds: [] }; + const result = goToSearchResultFactory(match)(ctx); + + expect(result).toBe(true); + expect(scrollToPosition).toHaveBeenCalledWith(10, { block: 'center' }); + // Async and DOM fallback should NOT fire when sync succeeds + expect(scrollToPositionAsync).not.toHaveBeenCalled(); + expect(ctx._domNode.scrollIntoView).not.toHaveBeenCalled(); + }); + + it('falls back to DOM scroll when sync scrollToPosition fails', () => { + const scrollToPosition = vi.fn(() => false); + const scrollToPositionAsync = vi.fn(); + const ctx = createEditorContext({ + presentationEditor: { scrollToPosition, scrollToPositionAsync }, + }); + + const match = { from: 10, to: 20, text: 'hello', id: '1', ranges: [], trackerIds: [] }; + const result = goToSearchResultFactory(match)(ctx); + + expect(result).toBe(true); + expect(scrollToPosition).toHaveBeenCalled(); + // Async should fire for virtualized pages + expect(scrollToPositionAsync).toHaveBeenCalledWith(10, { block: 'center' }); + // DOM fallback should also fire + expect(ctx._domNode.scrollIntoView).toHaveBeenCalledWith({ + block: 'center', + inline: 'nearest', + }); + }); + + it('falls back to DOM scroll when no presentation editor exists', () => { + const ctx = createEditorContext({ presentationEditor: null }); + + const match = { from: 10, to: 20, text: 'hello', id: '1', ranges: [], trackerIds: [] }; + const result = goToSearchResultFactory(match)(ctx); + + expect(result).toBe(true); + expect(ctx._domNode.scrollIntoView).toHaveBeenCalledWith({ + block: 'center', + inline: 'nearest', + }); + }); + + it('does not produce unhandled rejection when async scroll rejects', async () => { + const scrollToPosition = vi.fn(() => false); + const scrollToPositionAsync = vi.fn(() => Promise.reject(new Error('page disappeared'))); + const ctx = createEditorContext({ + presentationEditor: { scrollToPosition, scrollToPositionAsync }, + }); + + const match = { from: 10, to: 20, text: 'hello', id: '1', ranges: [], trackerIds: [] }; + const result = goToSearchResultFactory(match)(ctx); + + expect(result).toBe(true); + expect(scrollToPositionAsync).toHaveBeenCalledWith(10, { block: 'center' }); + // Give the microtask queue a tick so the .catch() runs — no unhandled rejection + await new Promise((r) => setTimeout(r, 0)); + }); + + it('fires async scroll when sync is unavailable but async exists', () => { + const scrollToPositionAsync = vi.fn(); + const ctx = createEditorContext({ + presentationEditor: { scrollToPositionAsync }, + }); + + const match = { from: 10, to: 20, text: 'hello', id: '1', ranges: [], trackerIds: [] }; + const result = goToSearchResultFactory(match)(ctx); + + expect(result).toBe(true); + // scrollToPosition is undefined, so sync returns false (via ?. → undefined → ?? false) + expect(scrollToPositionAsync).toHaveBeenCalledWith(10, { block: 'center' }); + // DOM fallback should also fire since sync didn't succeed + expect(ctx._domNode.scrollIntoView).toHaveBeenCalled(); + }); +}); From 6b7461a4357c04c08e3ad150239a324489cc7c9a Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 28 Feb 2026 14:38:19 -0800 Subject: [PATCH 3/5] fix(super-editor): improve position tracker, restore reliable search result scrolling --- .../super-editor/src/core/PositionTracker.ts | 278 +++++++++++++++++- .../src/extensions/search/search.js | 85 +++--- .../search/search.scrollBehavior.test.js | 77 ++++- .../src/tests/core/position-tracker.test.js | 129 +++++++- .../dev/components/sidebar/SidebarSearch.vue | 97 ++++++ 5 files changed, 630 insertions(+), 36 deletions(-) diff --git a/packages/super-editor/src/core/PositionTracker.ts b/packages/super-editor/src/core/PositionTracker.ts index a969b24b19..54d683bd16 100644 --- a/packages/super-editor/src/core/PositionTracker.ts +++ b/packages/super-editor/src/core/PositionTracker.ts @@ -1,5 +1,6 @@ import type { Transaction } from 'prosemirror-state'; -import { Plugin, PluginKey } from 'prosemirror-state'; +import { Plugin, PluginKey, TextSelection } from 'prosemirror-state'; +import type { Node as ProseMirrorNode } from 'prosemirror-model'; import { Decoration, DecorationSet } from 'prosemirror-view'; import { v4 as uuidv4 } from 'uuid'; @@ -21,6 +22,46 @@ export type ResolvedRange = { spec: TrackedRangeSpec; }; +type TrackablePosition = { + blockId: string; + offset: number; +}; + +type TrackableInlineAnchor = { + start: TrackablePosition; + end: TrackablePosition; +}; + +type TrackableNodeAddress = + | { + kind: 'inline'; + anchor: TrackableInlineAnchor; + } + | { + kind: 'block'; + nodeId: string; + nodeType?: string; + }; + +type TrackableFindNodeItem = { + address?: TrackableNodeAddress; +}; + +type TrackNodeInput = TrackableNodeAddress | TrackableFindNodeItem; + +type ResolvedBlockCandidate = { + node: ProseMirrorNode; + pos: number; +}; + +type OffsetRange = { + start: number; + end: number; +}; + +const DEFAULT_TRACKED_NODE_TYPE = 'tracked-node'; +type ScrollBlock = 'start' | 'center' | 'end' | 'nearest'; + export type PositionTrackerState = { decorations: DecorationSet; generation: number; @@ -33,6 +74,140 @@ type PositionTrackerMeta = export const positionTrackerKey = new PluginKey('positionTracker'); +function getNodeIdCandidates(node: ProseMirrorNode): string[] { + const attrs = (node.attrs ?? {}) as Record; + const candidateFields = ['paraId', 'sdBlockId', 'blockId', 'id', 'uuid'] as const; + const ids: string[] = []; + + for (const field of candidateFields) { + const value = attrs[field]; + if (typeof value === 'string' && value.length > 0) { + ids.push(value); + } + } + + return ids; +} + +function findBlockCandidateById(doc: ProseMirrorNode, blockId: string): ResolvedBlockCandidate | null { + let match: ResolvedBlockCandidate | null = null; + let isAmbiguous = false; + + doc.descendants((node, pos) => { + if (!node.isBlock) return; + const ids = getNodeIdCandidates(node); + if (!ids.includes(blockId)) return; + + if (match) { + isAmbiguous = true; + return false; + } + + match = { node, pos }; + return; + }); + + if (isAmbiguous) return null; + return match; +} + +function resolveSegmentPosition( + targetOffset: number, + segmentStart: number, + segmentLength: number, + docFrom: number, + docTo: number, +): number { + if (segmentLength <= 1) { + return targetOffset <= segmentStart ? docFrom : docTo; + } + return docFrom + (targetOffset - segmentStart); +} + +function resolveOffsetsInBlock( + blockNode: ProseMirrorNode, + blockPos: number, + range: OffsetRange, +): { from: number; to: number } | null { + if (range.start < 0 || range.end < range.start) return null; + + let flattenedOffset = 0; + let fromPos: number | undefined; + let toPos: number | undefined; + + const advanceSegment = (segmentLength: number, docFrom: number, docTo: number) => { + const segmentStart = flattenedOffset; + const segmentEnd = flattenedOffset + segmentLength; + + if (fromPos == null && range.start <= segmentEnd) { + fromPos = resolveSegmentPosition(range.start, segmentStart, segmentLength, docFrom, docTo); + } + if (toPos == null && range.end <= segmentEnd) { + toPos = resolveSegmentPosition(range.end, segmentStart, segmentLength, docFrom, docTo); + } + + flattenedOffset = segmentEnd; + }; + + const walkNodeContent = (node: ProseMirrorNode, contentStart: number) => { + let isFirstChild = true; + let childOffset = 0; + + for (let i = 0; i < node.childCount; i += 1) { + const child = node.child(i); + const childPos = contentStart + childOffset; + + if (child.isBlock && !isFirstChild) { + advanceSegment(1, childPos, childPos + 1); + } + + walkNode(child, childPos); + childOffset += child.nodeSize; + isFirstChild = false; + } + }; + + const walkNode = (node: ProseMirrorNode, docPos: number) => { + if (node.isText) { + const text = node.text ?? ''; + if (text.length > 0) { + advanceSegment(text.length, docPos, docPos + text.length); + } + return; + } + + if (node.isLeaf) { + advanceSegment(1, docPos, docPos + node.nodeSize); + return; + } + + walkNodeContent(node, docPos + 1); + }; + + walkNodeContent(blockNode, blockPos + 1); + + if (flattenedOffset === 0 && range.start === 0 && range.end === 0) { + const anchor = blockPos + 1; + return { from: anchor, to: anchor }; + } + + if (range.end > flattenedOffset) return null; + if (fromPos == null || toPos == null) return null; + return { from: fromPos, to: toPos }; +} + +function getTrackableAddress(input: TrackNodeInput): TrackableNodeAddress | null { + if (!input || typeof input !== 'object') return null; + if ('kind' in input && (input.kind === 'inline' || input.kind === 'block')) { + return input as TrackableNodeAddress; + } + if ('address' in input && input.address && typeof input.address === 'object') { + const address = input.address as TrackableNodeAddress; + if (address.kind === 'inline' || address.kind === 'block') return address; + } + return null; +} + export function createPositionTrackerPlugin(): Plugin { return new Plugin({ key: positionTrackerKey, @@ -92,6 +267,30 @@ export class PositionTracker { return positionTrackerKey.getState(this.#editor.state) ?? null; } + #resolveTrackNodeAddress(address: TrackableNodeAddress): { from: number; to: number } | null { + const doc = this.#editor?.state?.doc; + if (!doc) return null; + + if (address.kind === 'inline') { + const { start, end } = address.anchor; + if (!start || !end || start.blockId !== end.blockId) return null; + + const block = findBlockCandidateById(doc, start.blockId); + if (!block) return null; + + return resolveOffsetsInBlock(block.node, block.pos, { + start: start.offset, + end: end.offset, + }); + } + + const block = findBlockCandidateById(doc, address.nodeId); + if (!block) return null; + + const anchor = block.pos + 1; + return { from: anchor, to: anchor }; + } + track(from: number, to: number, spec: Omit): string { const id = uuidv4(); if (!this.#editor?.state) return id; @@ -135,6 +334,45 @@ export class PositionTracker { return ids; } + trackNode(input: TrackNodeInput, spec?: Omit): string | null { + const [trackedId] = this.trackNodes([input], spec); + return trackedId ?? null; + } + + trackNodes(inputs: TrackNodeInput[], spec?: Omit): Array { + if (!Array.isArray(inputs) || inputs.length === 0) return []; + + const trackSpec = { type: DEFAULT_TRACKED_NODE_TYPE, ...(spec ?? {}) }; + const pendingRanges: Array<{ from: number; to: number; spec: Omit }> = []; + const pendingInputIndexes: number[] = []; + const results: Array = Array.from({ length: inputs.length }, () => null); + + for (let index = 0; index < inputs.length; index += 1) { + const address = getTrackableAddress(inputs[index]); + if (!address) continue; + + const resolved = this.#resolveTrackNodeAddress(address); + if (!resolved) continue; + + pendingRanges.push({ + from: resolved.from, + to: resolved.to, + spec: trackSpec, + }); + pendingInputIndexes.push(index); + } + + if (pendingRanges.length === 0) return results; + + const trackedIds = this.trackMany(pendingRanges); + for (let i = 0; i < pendingInputIndexes.length; i += 1) { + const inputIndex = pendingInputIndexes[i]; + results[inputIndex] = trackedIds[i] ?? null; + } + + return results; + } + untrack(id: string): void { if (!this.#editor?.state) return; const tr = this.#editor.state.tr @@ -226,6 +464,44 @@ export class PositionTracker { }); } + goToTracked(id: string, options?: { block?: ScrollBlock }): boolean { + const resolved = this.resolve(id); + if (!resolved) return false; + + const from = Math.max(0, Math.min(resolved.from, this.#editor.state.doc.content.size)); + const to = Math.max(from, Math.min(resolved.to, this.#editor.state.doc.content.size)); + const block = options?.block ?? 'center'; + + if (this.#editor.commands?.setTextSelection) { + this.#editor.commands.setTextSelection({ from, to }); + } else if (this.#editor.state) { + const tr = this.#editor.state.tr + .setSelection(TextSelection.create(this.#editor.state.doc, from, to)) + .scrollIntoView(); + this.#editor.dispatch(tr); + } + + const presentationEditor = this.#editor.presentationEditor; + const didPresentationScroll = presentationEditor?.scrollToPosition?.(from, { block }) ?? false; + + if (!didPresentationScroll) { + Promise.resolve(presentationEditor?.scrollToPositionAsync?.(from, { block })).catch(() => {}); + + try { + const { node } = this.#editor.view?.domAtPos(from) ?? { node: null }; + if (typeof Element !== 'undefined' && node instanceof Element) { + node.scrollIntoView({ block, inline: 'nearest' }); + } else if ((node as Node | null)?.parentElement) { + (node as Node).parentElement?.scrollIntoView({ block, inline: 'nearest' }); + } + } catch { + // Ignore scroll failures in environments with incomplete DOM APIs. + } + } + + return true; + } + get generation(): number { return this.#getState()?.generation ?? 0; } diff --git a/packages/super-editor/src/extensions/search/search.js b/packages/super-editor/src/extensions/search/search.js index cd3f0dc061..20f0807560 100644 --- a/packages/super-editor/src/extensions/search/search.js +++ b/packages/super-editor/src/extensions/search/search.js @@ -25,6 +25,32 @@ export const getCustomSearchDecorations = (state) => { }; const isRegExp = (value) => Object.prototype.toString.call(value) === '[object RegExp]'; +const SEARCH_POSITION_TRACKER_TYPE = 'search-match'; + +const resolveMatchSelectionRange = (match, positionTracker) => { + if (!match) return { from: undefined, to: undefined }; + + if (positionTracker?.resolve && Array.isArray(match.trackerIds) && match.trackerIds.length > 0) { + const resolved = positionTracker.resolve(match.trackerIds[0]); + if (resolved) { + return { from: resolved.from, to: resolved.to }; + } + } + + if (match?.ranges && match.ranges.length > 0) { + return { from: match.ranges[0].from, to: match.ranges[0].to }; + } + + if (positionTracker?.resolve && match?.id) { + const resolved = positionTracker.resolve(match.id); + if (resolved) { + return { from: resolved.from, to: resolved.to }; + } + } + + return { from: match.from, to: match.to }; +}; + const resolveInlineTextPosition = (doc, position, direction) => { const docSize = doc.content.size; if (!Number.isFinite(position) || position < 0 || position > docSize) { @@ -240,8 +266,8 @@ export const Search = Extension.create({ const searchResults = this.storage?.searchResults; if (Array.isArray(searchResults) && searchResults.length > 0) { const firstMatch = searchResults[0]; - const from = firstMatch.ranges?.[0]?.from ?? firstMatch.from; - const to = firstMatch.ranges?.[0]?.to ?? firstMatch.to; + const positionTracker = getPositionTracker(editor); + const { from, to } = resolveMatchSelectionRange(firstMatch, positionTracker); if (typeof from !== 'number' || typeof to !== 'number') { return false; @@ -353,6 +379,10 @@ export const Search = Extension.create({ caseSensitive = typeof options?.caseSensitive === 'boolean' ? options.caseSensitive : false; } + const positionTracker = getPositionTracker(editor); + // Keep tracker set bounded to current search results only. + positionTracker?.untrackByType?.(SEARCH_POSITION_TRACKER_TYPE); + // Ensure search index is valid const searchIndex = this.storage.searchIndex; searchIndex.ensureValid(state.doc); @@ -382,6 +412,24 @@ export const Search = Extension.create({ trackerIds: [], }; + if (positionTracker?.trackMany) { + const trackedRanges = ranges.map((range, rangeIndex) => ({ + from: range.from, + to: range.to, + spec: { + type: SEARCH_POSITION_TRACKER_TYPE, + metadata: { + rangeIndex, + }, + }, + })); + const trackerIds = positionTracker.trackMany(trackedRanges); + if (trackerIds.length > 0) { + match.trackerIds = trackerIds; + match.id = trackerIds[0]; + } + } + resultMatches.push(match); } @@ -410,37 +458,8 @@ export const Search = Extension.create({ const doc = state.doc; const highlights = getMatchHighlights(state); - let from, to; - - // Handle multi-range matches (cross-paragraph) - if (match?.ranges && match.ranges.length > 0 && match?.trackerIds && match.trackerIds.length > 0) { - // Resolve the first tracked range for selection - if (positionTracker?.resolve && match.trackerIds[0]) { - const resolved = positionTracker.resolve(match.trackerIds[0]); - if (resolved) { - from = resolved.from; - to = resolved.to; - } - } - - // Fallback to stored range if tracking failed - if (from === undefined) { - from = match.ranges[0].from; - to = match.ranges[0].to; - } - } else { - // Single range match (backward compatibility) - from = match.from; - to = match.to; - - if (positionTracker?.resolve && match?.id) { - const resolved = positionTracker.resolve(match.id); - if (resolved) { - from = resolved.from; - to = resolved.to; - } - } - } + let { from, to } = resolveMatchSelectionRange(match, positionTracker); + if (typeof from !== 'number' || typeof to !== 'number') return false; // Normalize the range to handle transparent inline nodes const normalized = resolveSearchRange({ diff --git a/packages/super-editor/src/extensions/search/search.scrollBehavior.test.js b/packages/super-editor/src/extensions/search/search.scrollBehavior.test.js index 9c5b6ee188..49f52d6379 100644 --- a/packages/super-editor/src/extensions/search/search.scrollBehavior.test.js +++ b/packages/super-editor/src/extensions/search/search.scrollBehavior.test.js @@ -58,6 +58,7 @@ function createEditorContext(overrides = {}) { const doc = { content: { size: 200 }, nodeSize: 200, + textBetween: vi.fn(() => 'hello'), resolve: vi.fn(() => ({ node: vi.fn(() => null), parent: { type: { name: 'paragraph' } }, @@ -83,7 +84,7 @@ function createEditorContext(overrides = {}) { domAtPos: vi.fn(() => ({ node: mockNode })), }, presentationEditor: overrides.presentationEditor ?? null, - positionTracker: { resolve: vi.fn(() => null) }, + positionTracker: overrides.positionTracker ?? { resolve: vi.fn(() => null) }, storage: { positionTracker: { tracker: null } }, }, // Expose the mock node for assertions @@ -187,4 +188,78 @@ describe('goToSearchResult — scroll behavior', () => { // DOM fallback should also fire since sync didn't succeed expect(ctx._domNode.scrollIntoView).toHaveBeenCalled(); }); + + it('resolves tracked positions when stored match positions are stale', () => { + const positionTracker = { + resolve: vi.fn(() => ({ from: 60, to: 65 })), + }; + const ctx = createEditorContext({ presentationEditor: null, positionTracker }); + + const staleMatch = { + from: 10, + to: 15, + text: 'hello', + id: 'old-id', + ranges: [{ from: 10, to: 15 }], + trackerIds: ['tracked-id'], + }; + + const result = goToSearchResultFactory(staleMatch)(ctx); + + expect(result).toBe(true); + expect(positionTracker.resolve).toHaveBeenCalledWith('tracked-id'); + expect(ctx.state.tr.setSelection).toHaveBeenCalled(); + }); +}); + +describe('search command — position tracking', () => { + let searchCommandFactory; + + beforeEach(() => { + const searchIndex = { + ensureValid: vi.fn(), + search: vi.fn(() => [{ start: 0, end: 5 }]), + offsetRangeToDocRanges: vi.fn(() => [{ from: 10, to: 15 }]), + }; + const ext = Search.config; + const commands = ext.addCommands.call({ + editor: null, + storage: { + searchResults: [], + highlightEnabled: true, + searchIndex, + }, + }); + searchCommandFactory = commands.search; + }); + + it('tracks result ranges and uses tracker id as match id', () => { + const positionTracker = { + untrackByType: vi.fn(), + trackMany: vi.fn(() => ['tracked-match-id']), + resolve: vi.fn(() => null), + }; + const ctx = createEditorContext({ positionTracker }); + + const results = searchCommandFactory('hello')({ + state: ctx.state, + dispatch: ctx.dispatch, + editor: ctx.editor, + }); + + expect(positionTracker.untrackByType).toHaveBeenCalledWith('search-match'); + expect(positionTracker.trackMany).toHaveBeenCalledWith([ + { + from: 10, + to: 15, + spec: { + type: 'search-match', + metadata: { rangeIndex: 0 }, + }, + }, + ]); + expect(results).toHaveLength(1); + expect(results[0].trackerIds).toEqual(['tracked-match-id']); + expect(results[0].id).toBe('tracked-match-id'); + }); }); diff --git a/packages/super-editor/src/tests/core/position-tracker.test.js b/packages/super-editor/src/tests/core/position-tracker.test.js index db1c642e0a..7444acefc8 100644 --- a/packages/super-editor/src/tests/core/position-tracker.test.js +++ b/packages/super-editor/src/tests/core/position-tracker.test.js @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { createDocxTestEditor } from '../helpers/editor-test-utils.js'; import { EditorState } from 'prosemirror-state'; import { positionTrackerKey } from '@core/PositionTracker.js'; @@ -658,6 +658,133 @@ describe('PositionTracker', () => { }); }); + describe('trackNode() and goToTracked()', () => { + it('should track a find() item via trackNode', () => { + const editor = createDocxTestEditor(); + + try { + const { doc, paragraph, run } = editor.schema.nodes; + const linkMark = editor.schema.marks.link.create({ href: 'http://www.google.com' }); + const testDoc = doc.create(null, [ + paragraph.create({ paraId: '4652C010' }, [ + run.create(null, [editor.schema.text('Google', [linkMark]), editor.schema.text(' docs')]), + ]), + ]); + + const baseState = EditorState.create({ + schema: editor.schema, + doc: testDoc, + plugins: editor.state.plugins, + }); + editor.setState(baseState); + + const tracker = editor.positionTracker; + const findItem = { + address: { + kind: 'inline', + nodeType: 'hyperlink', + anchor: { + start: { blockId: '4652C010', offset: 0 }, + end: { blockId: '4652C010', offset: 6 }, + }, + }, + node: { + nodeType: 'hyperlink', + kind: 'inline', + properties: { href: 'http://www.google.com' }, + }, + }; + + const id = tracker.trackNode(findItem, { type: 'sidebar-link' }); + + expect(id).toBeTypeOf('string'); + const resolved = tracker.resolve(id); + expect(resolved).not.toBeNull(); + expect(editor.state.doc.textBetween(resolved.from, resolved.to)).toBe('Google'); + expect(resolved.spec.type).toBe('sidebar-link'); + } finally { + editor.destroy(); + } + }); + + it('should return null from trackNode when the find item cannot be resolved', () => { + const editor = createDocxTestEditor(); + + try { + const { doc, paragraph, run } = editor.schema.nodes; + const testDoc = doc.create(null, [paragraph.create(null, [run.create(null, [editor.schema.text('hello')])])]); + + const baseState = EditorState.create({ + schema: editor.schema, + doc: testDoc, + plugins: editor.state.plugins, + }); + editor.setState(baseState); + + const tracker = editor.positionTracker; + const unresolvedItem = { + address: { + kind: 'inline', + anchor: { + start: { blockId: 'missing-block', offset: 0 }, + end: { blockId: 'missing-block', offset: 5 }, + }, + }, + }; + + expect(tracker.trackNode(unresolvedItem)).toBeNull(); + } finally { + editor.destroy(); + } + }); + + it('should navigate tracked ranges with goToTracked', () => { + const editor = createDocxTestEditor(); + + try { + const { doc, paragraph, run } = editor.schema.nodes; + const testDoc = doc.create(null, [ + paragraph.create(null, [run.create(null, [editor.schema.text('hello world')])]), + ]); + + const baseState = EditorState.create({ + schema: editor.schema, + doc: testDoc, + plugins: editor.state.plugins, + }); + editor.setState(baseState); + + const tracker = editor.positionTracker; + let worldFrom = null; + let worldTo = null; + editor.state.doc.descendants((node, pos) => { + if (!node.isText) return; + const index = (node.text ?? '').indexOf('world'); + if (index === -1 || worldFrom != null) return; + worldFrom = pos + index; + worldTo = pos + index + 5; + }); + expect(worldFrom).toBeTypeOf('number'); + expect(worldTo).toBeTypeOf('number'); + const id = tracker.track(worldFrom, worldTo, { type: 'search' }); + + const scrollToPosition = vi.fn(() => true); + editor.presentationEditor = { + scrollToPosition, + }; + + const didNavigate = tracker.goToTracked(id); + + expect(didNavigate).toBe(true); + expect(editor.state.selection.from).toBe(worldFrom); + expect(editor.state.selection.to).toBe(worldTo); + expect(scrollToPosition).toHaveBeenCalledWith(worldFrom, { block: 'center' }); + } finally { + editor.destroy(); + } + }); + }); + describe('generation counter', () => { it('should increment generation on document changes', () => { const editor = createDocxTestEditor(); diff --git a/packages/superdoc/src/dev/components/sidebar/SidebarSearch.vue b/packages/superdoc/src/dev/components/sidebar/SidebarSearch.vue index 82f3cf82dd..ba7c3a9e72 100644 --- a/packages/superdoc/src/dev/components/sidebar/SidebarSearch.vue +++ b/packages/superdoc/src/dev/components/sidebar/SidebarSearch.vue @@ -4,6 +4,8 @@ import { ref } from 'vue'; const query = ref(''); const results = ref([]); const hasSearched = ref(false); +const links = ref([]); +const hasScannedLinks = ref(false); const emit = defineEmits(['close']); @@ -32,6 +34,61 @@ const goToResult = (match) => { return; } }; + +const getLinkLabel = (item, index) => { + const href = item?.node?.properties?.href; + const anchor = item?.node?.properties?.anchor; + if (typeof href === 'string' && href.length > 0) return href; + if (typeof anchor === 'string' && anchor.length > 0) return `#${anchor}`; + return `Link ${index + 1}`; +}; + +const findLinks = () => { + const editor = window.editor; + const find = editor?.doc?.find; + if (typeof find !== 'function') { + links.value = []; + hasScannedLinks.value = true; + return; + } + + const findResult = find({ + select: { type: 'node', nodeType: 'hyperlink', kind: 'inline' }, + includeNodes: true, + limit: 1000, + }); + const items = Array.isArray(findResult?.items) ? findResult.items : []; + + links.value = items.map((item, index) => ({ + id: item?.id ?? `link-${index}`, + label: getLinkLabel(item, index), + item, + trackerId: editor?.positionTracker?.trackNode?.(item, { + type: 'sidebar-link', + metadata: { source: 'superdoc-dev-links', index }, + }), + })); + hasScannedLinks.value = true; +}; + +const goToLink = (link) => { + const editor = window.editor; + if (!editor) return; + + const tracker = editor?.positionTracker; + if (typeof tracker?.goToTracked !== 'function') return; + + let trackerId = link?.trackerId ?? null; + if (!trackerId && typeof tracker.trackNode === 'function' && link?.item) { + trackerId = tracker.trackNode(link.item, { + type: 'sidebar-link', + metadata: { source: 'superdoc-dev-links' }, + }); + } + if (!trackerId) return; + + tracker.goToTracked(trackerId); +}; @@ -173,6 +251,25 @@ const goToResult = (match) => { gap: 8px; } +.dev-sidebar__section { + display: grid; + gap: 8px; +} + +.dev-sidebar__section-header { + display: flex; + align-items: center; + justify-content: space-between; + gap: 8px; +} + +.dev-sidebar__section-title { + margin: 0; + font-size: 13px; + font-weight: 700; + color: #334155; +} + .dev-sidebar__result { display: grid; grid-template-columns: auto minmax(0, 1fr); From fd2ee2412580fce8096b1099149b77a7b6f5ca89 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 28 Feb 2026 14:41:37 -0800 Subject: [PATCH 4/5] docs: add stable navigation docs --- apps/docs/docs.json | 1 + .../docs/guides/general/stable-navigation.mdx | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 apps/docs/guides/general/stable-navigation.mdx diff --git a/apps/docs/docs.json b/apps/docs/docs.json index 3d53a1f80c..dd6772ca5f 100644 --- a/apps/docs/docs.json +++ b/apps/docs/docs.json @@ -227,6 +227,7 @@ "group": "Guides", "pages": [ "guides/general/storage", + "guides/general/stable-navigation", { "group": "Collaboration", "pages": [ diff --git a/apps/docs/guides/general/stable-navigation.mdx b/apps/docs/guides/general/stable-navigation.mdx new file mode 100644 index 0000000000..5df043ed31 --- /dev/null +++ b/apps/docs/guides/general/stable-navigation.mdx @@ -0,0 +1,36 @@ +--- +title: Stable navigation after edits +sidebarTitle: Stable navigation +description: Find nodes once and navigate to them reliably after document edits. +--- + +When users edit a document, stored positions can drift. +Use `PositionTracker` so navigation targets stay stable. + +## Hyperlinks example + +```javascript +// 1) Find hyperlink nodes +const found = editor.doc.find({ + select: { type: 'node', nodeType: 'hyperlink', kind: 'inline' }, +}); + +// 2) Track each found node +const links = found.items.map((item) => ({ + item, + trackerId: editor.positionTracker.trackNode(item), +})); + +// 3) Navigate later (for example, from a sidebar click) +function goToLink(link) { + if (!link?.trackerId) return; + editor.positionTracker.goToTracked(link.trackerId); +} +``` + +## Best practices + +- Track results right after `find()`. +- Store `trackerId` in UI state, not raw positions. +- Re-run `find()` and rebuild tracked IDs when refreshing your list. +- Handle missing targets gracefully (`goToTracked` can return `false` if content was removed). From 1a911e62c0a94fc3c7112a0191d07123d1f6a710 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 28 Feb 2026 14:52:34 -0800 Subject: [PATCH 5/5] chore: fix test --- .../enter-page-boundary-line-count.spec.ts | 86 ++++++++++++------- 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/tests/behavior/tests/basic-commands/enter-page-boundary-line-count.spec.ts b/tests/behavior/tests/basic-commands/enter-page-boundary-line-count.spec.ts index ea08a43889..78e855b0a0 100644 --- a/tests/behavior/tests/basic-commands/enter-page-boundary-line-count.spec.ts +++ b/tests/behavior/tests/basic-commands/enter-page-boundary-line-count.spec.ts @@ -6,6 +6,50 @@ import { test } from '../../fixtures/superdoc.js'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const MARKDOWN_PATH = path.resolve(__dirname, 'fixtures/enter-page-boundary.md'); +type HeadingPageCheck = { + ok: boolean; + headingLineIndex?: number; + previousLineText?: string | null; + firstLineText?: string | null; + reason?: string; +}; + +const evaluateHeadingPagePosition = async (superdoc: any, needle: string): Promise => { + return superdoc.page.evaluate((headingNeedle) => { + const normalize = (value: string | null | undefined) => (value ?? '').replace(/\s+/g, ' ').trim(); + const lines = Array.from(document.querySelectorAll('.superdoc-line')) as HTMLElement[]; + const headingLine = lines.find((line) => normalize(line.textContent).toLowerCase() === headingNeedle.toLowerCase()); + if (!headingLine) { + return { ok: false, reason: 'heading line not found in rendered DOM' }; + } + + const page = headingLine.closest('.superdoc-page'); + if (!page) { + return { ok: false, reason: 'heading line is not inside a .superdoc-page element' }; + } + + const bodyLines = Array.from(page.querySelectorAll('.superdoc-line')).filter((line) => { + const element = line as HTMLElement; + return !element.closest('.superdoc-page-header') && !element.closest('.superdoc-page-footer'); + }) as HTMLElement[]; + + const headingLineIndex = bodyLines.findIndex((line) => line === headingLine); + if (headingLineIndex < 0) { + return { ok: false, reason: 'heading line not found among page body lines' }; + } + + const previousLineText = headingLineIndex > 0 ? normalize(bodyLines[headingLineIndex - 1].textContent) : null; + const firstLineText = bodyLines[0] ? normalize(bodyLines[0].textContent) : null; + + return { + ok: headingLineIndex === 0, + headingLineIndex, + previousLineText, + firstLineText, + }; + }, needle); +}; + test('enter at heading boundary does not insert extra blank line above heading on next page', async ({ superdoc }) => { const markdown = fs.readFileSync(MARKDOWN_PATH, 'utf8'); @@ -50,6 +94,14 @@ test('enter at heading boundary does not insert extra blank line above heading o throw new Error(`Unable to find heading text "${headingText}" in document.`); } + const preCheck = await evaluateHeadingPagePosition(superdoc, headingText); + test.skip( + !preCheck.ok, + `Precondition unmet: heading is not at page boundary in this environment (index=${String(preCheck.headingLineIndex)}, first="${String( + preCheck.firstLineText, + )}").`, + ); + await superdoc.page.evaluate(() => { const editor = (window as any).editor; editor?.commands?.focus?.(); @@ -58,39 +110,7 @@ test('enter at heading boundary does not insert extra blank line above heading o await superdoc.press('Enter'); await superdoc.waitForStable(); - const pageCheck = await superdoc.page.evaluate((needle) => { - const normalize = (value: string | null | undefined) => (value ?? '').replace(/\s+/g, ' ').trim(); - const lines = Array.from(document.querySelectorAll('.superdoc-line')) as HTMLElement[]; - const headingLine = lines.find((line) => normalize(line.textContent).toLowerCase() === needle.toLowerCase()); - if (!headingLine) { - return { ok: false, reason: 'heading line not found in rendered DOM' }; - } - - const page = headingLine.closest('.superdoc-page'); - if (!page) { - return { ok: false, reason: 'heading line is not inside a .superdoc-page element' }; - } - - const bodyLines = Array.from(page.querySelectorAll('.superdoc-line')).filter((line) => { - const element = line as HTMLElement; - return !element.closest('.superdoc-page-header') && !element.closest('.superdoc-page-footer'); - }) as HTMLElement[]; - - const headingLineIndex = bodyLines.findIndex((line) => line === headingLine); - if (headingLineIndex < 0) { - return { ok: false, reason: 'heading line not found among page body lines' }; - } - - const previousLineText = headingLineIndex > 0 ? normalize(bodyLines[headingLineIndex - 1].textContent) : null; - const firstLineText = bodyLines[0] ? normalize(bodyLines[0].textContent) : null; - - return { - ok: headingLineIndex === 0, - headingLineIndex, - previousLineText, - firstLineText, - }; - }, headingText); + const pageCheck = await evaluateHeadingPagePosition(superdoc, headingText); if (!pageCheck.ok) { throw new Error(