From 6de21f6f7ceaa46357e003b01b9f45f5fe751ec6 Mon Sep 17 00:00:00 2001 From: bjohas Date: Thu, 14 May 2026 09:02:48 +0100 Subject: [PATCH 1/6] fix(super-editor): resolve child element in Editor.getElementAtPos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `view.domAtPos(pos)` returns `{node, offset}` where `node` is an element parent and `offset` is the child index when the position sits between block children. The previous implementation returned the parent directly, which for the editor root collapses every position into "the entire document" — `scrollIntoView()` on that element always lands at the top of the editor regardless of which heading or paragraph was requested. Resolve to `parent.childNodes[offset]` (clamped) when the returned node is an element parent. The text-node branch is unchanged. This fixes web/flow-layout `getElementAtPos` for positions on block boundaries; the presentation-editor branch isn't touched. Verified with a probe that calls the body-editor `getElementAtPos` for known heading positions (2725, 3513, 4477, 13264) — before this fix all four returned the same ProseMirror root DIV; after, each returns the actual heading's wrapper element. Co-Authored-By: Claude Opus 4.7 --- .../src/editors/v1/core/Editor.ts | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/Editor.ts b/packages/super-editor/src/editors/v1/core/Editor.ts index df697952dc..2f4c85918d 100644 --- a/packages/super-editor/src/editors/v1/core/Editor.ts +++ b/packages/super-editor/src/editors/v1/core/Editor.ts @@ -1718,11 +1718,30 @@ export class Editor extends EventEmitter { const clampedPos = Math.max(0, Math.min(pos, maxPos)); try { - const { node } = this.view.domAtPos(clampedPos); - if (node && node.nodeType === 1) { - return node as HTMLElement; + // ProseMirror's domAtPos returns either: + // - { node: , offset: }, or + // - { node: , offset: } when the position is + // between block children. + // The previous version returned the parent in the second case, which + // for the editor root means the entire document — scrolling that into + // view always lands at the top. Resolve to the actual child element + // when the returned node is an element parent. + const { node, offset } = this.view.domAtPos(clampedPos); + if (!node) return null; + + if (node.nodeType === 1) { + const parent = node as Element; + if (parent.childNodes?.length) { + const idx = Math.min(Math.max(0, offset), parent.childNodes.length - 1); + const child = parent.childNodes[idx]; + if (child) { + if (child.nodeType === 1) return child as HTMLElement; + if (child.nodeType === 3) return (child as Node).parentElement; + } + } + return parent as HTMLElement; } - if (node && node.nodeType === 3) { + if (node.nodeType === 3) { return node.parentElement; } return node?.parentElement ?? null; From e44c62748a436edd91f28f1f0c4931d258fd8703 Mon Sep 17 00:00:00 2001 From: bjohas Date: Thu, 14 May 2026 09:03:15 +0100 Subject: [PATCH 2/6] feat(superdoc): fix scrollToComment + scrollToElement, add scrollToHeading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related fixes to the public scroll-to-X API surface, motivated by an external integration where comment-panel click → editor scroll was silently broken in flow layout and unreliable in paginated layout. fix(superdoc): scrollToComment delegates to scrollToElement The previous implementation hand-rolled a DOM-only lookup (`root.querySelector('[data-comment-ids*="..."]')` → `scrollIntoView`). This had two failure modes: 1. Paginated/presentation layout virtualises pages — comment highlights on offscreen pages aren't in the DOM, so the selector misses and the call returns false even though the comment exists in the document. 2. When two `commentMark`s share a position, SuperDoc emits a single `.sd-editor-comment-highlight` element. Lookups for the suppressed thread silently fail in either layout. Route through the existing `scrollToElement` instead. It already does the right thing for both layouts (with the flow-layout fallback added below). The side-panel activation is preserved. fix(superdoc): scrollToElement falls back to body-editor in flow In flow / web layouts there is no `presentationEditor`, so `presentationEditor.scrollToElement` returned false unconditionally and the whole API was a no-op outside paginated mode. When the presentation path isn't available (or returns false), walk the ProseMirror doc directly: use `editor.commands.setCursorById` to resolve comment and tracked-change marks (it already does the right `findRangeById`-based lookup), then fall back to a one-pass attr-match for block IDs (`nodeId` / `sdBlockId` / `id` / `paraId`). Use `editor.getElementAtPos(pos)` for the DOM target so callers benefit from the companion `Editor.getElementAtPos` fix in the same branch. feat(superdoc): scrollToHeading(level, ordinal, options) New public API. Walks the ProseMirror doc counting headings of the requested level — recognises both the `heading` node type (level attr) and the OOXML import shape where headings are paragraphs with `paragraphProperties.styleId = 'HeadingN'` — and scrolls to the 1-based `ordinal`-th match. The walk targets a position INSIDE the heading paragraph's text content (the first text descendant), not the doc-level boundary before the paragraph. The presentation editor's layout fragment index only spans positions inside text content, so a boundary position misses every fragment and `scrollToPositionAsync` returns false. Walking one level in unblocks the print-mode path. Verified against a Playwright probe on a 4-comment, 41-heading test docx: flow comment-scroll went from 0/4 to 4/4 (`scrollToComment`); heading scroll works 9/9 in both layouts at levels 1–3, ordinals 1–3. Print-mode comment scroll is unchanged at 3/4 (one residual case where `scrollToPositionAsync` returns true but the painter doesn't actually scroll; tracked separately as it's not in this patch's scope). Co-Authored-By: Claude Opus 4.7 --- packages/superdoc/src/core/SuperDoc.js | 207 +++++++++++++++++++++++-- 1 file changed, 191 insertions(+), 16 deletions(-) diff --git a/packages/superdoc/src/core/SuperDoc.js b/packages/superdoc/src/core/SuperDoc.js index 59d9a729d1..a2c14469f6 100644 --- a/packages/superdoc/src/core/SuperDoc.js +++ b/packages/superdoc/src/core/SuperDoc.js @@ -1323,27 +1323,29 @@ export class SuperDoc extends EventEmitter { /** * Scroll the document to a given comment by id. * + * Delegates to {@link scrollToElement} so it works in both flow and + * paginated layouts. The previous implementation looked up the highlight + * span via `[data-comment-ids*=...]` and called `scrollIntoView()` on it + * directly — that fails in paginated/print mode (the painter virtualises + * pages so the highlight may not be in the DOM) and also fails for marks + * SuperDoc didn't emit a visible highlight for (e.g. two marks sharing a + * single position). The unified path walks the ProseMirror doc for the + * mark and dispatches to the presentation editor where available, + * falling back to the body editor in flow mode. + * * @param {string} commentId The comment id * @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition }} [options] - * @returns {boolean} Whether a matching element was found + * @returns {Promise} Whether the comment was found and scrolled to */ - scrollToComment(commentId, options = {}) { + async scrollToComment(commentId, options = {}) { const commentsConfig = this.config?.modules?.comments; - // `commentsConfig` can be `false | object | undefined`; `!commentsConfig` - // already covers both `false` and `undefined`, so the secondary - // `=== false` check below is redundant. if (!commentsConfig) return false; if (!commentId || typeof commentId !== 'string') return false; - const root = this.element || document; - const escaped = globalThis.CSS?.escape ? globalThis.CSS.escape(commentId) : commentId.replace(/"/g, '\\"'); - const element = root.querySelector(`[data-comment-ids*="${escaped}"]`); - if (!element) return false; - - const { behavior = 'smooth', block = 'start' } = options ?? {}; - element.scrollIntoView({ behavior, block }); + // Activate the thread in the side panel for visual continuity even if + // the scroll path subsequently bails. this.commentsStore?.setActiveComment?.(this, commentId); - return true; + return this.scrollToElement(commentId, options); } /** @@ -1371,7 +1373,13 @@ export class SuperDoc extends EventEmitter { * change entityId. The method resolves the element type automatically * and scrolls to it. * + * In paginated (presentation) layouts this delegates to the + * presentation editor's `scrollToElement`. In flow / web layouts the + * presentation editor doesn't apply, so we fall back to walking the + * ProseMirror doc directly and scrolling the body editor's view. + * * @param {string} elementId - The element's stable ID. + * @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition }} [options] * @returns {Promise} Whether the element was found and scrolled to. * * @example @@ -1381,13 +1389,180 @@ export class SuperDoc extends EventEmitter { * // Navigate to a comment by its entityId * await superdoc.scrollToElement('imported-25def254'); */ - async scrollToElement(elementId) { + async scrollToElement(elementId, options = {}) { + if (!elementId) return false; /** @type {RuntimeDocument[] | undefined} */ const storeDocs = this.superdocStore?.documents; if (!storeDocs?.length) return false; + const presentationEditor = storeDocs[0].getPresentationEditor?.(); - if (!presentationEditor?.scrollToElement) return false; - return presentationEditor.scrollToElement(elementId); + if (presentationEditor?.scrollToElement) { + const ok = await presentationEditor.scrollToElement(elementId); + if (ok) return true; + // Otherwise: presentationEditor may have returned false because layout + // state isn't active (flow/web mode masquerading as presentation). Fall + // through to the body-editor path. + } + + return this._scrollToElementInBodyEditor(elementId, options); + } + + /** + * Flow-layout fallback for {@link scrollToElement}. + * + * The body editor IS the visible view in flow mode, so we walk PM for the + * target position and use the editor's own DOM-by-position helper, then + * scroll the resulting element into view. Tries comment / tracked-change + * marks (via the existing `setCursorById` command) first, then falls back + * to block-level node IDs (paragraphs, headings, tables) by attribute + * matching. + * + * @param {string} elementId + * @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition }} [options] + * @returns {Promise} + * @private + */ + async _scrollToElementInBodyEditor(elementId, options = {}) { + const editor = this.activeEditor; + if (!editor?.state?.doc) return false; + + let pos = null; + + // 1. Try the comments-plugin command — handles commentMark, tracked + // change marks, and commentRangeStart/End nodes uniformly. + const setCursorById = editor.commands?.setCursorById; + if (typeof setCursorById === 'function') { + if (setCursorById(elementId, { preferredActiveThreadId: elementId })) { + pos = editor.state.selection?.from; + } + } + + // 2. Fall back to a single PM walk looking for matching block-level + // id attributes (nodeId / sdBlockId / id / paraId). + if (pos == null || !Number.isFinite(pos)) { + editor.state.doc.descendants((node, p) => { + if (pos != null) return false; + const a = node.attrs || {}; + const candidate = a.nodeId ?? a.sdBlockId ?? a.id ?? a.paraId; + if (candidate && candidate === elementId) { + pos = p; + return false; + } + }); + } + + if (pos == null || !Number.isFinite(pos)) return false; + + const targetEl = typeof editor.getElementAtPos === 'function' ? editor.getElementAtPos(pos) : null; + if (!targetEl?.scrollIntoView) return false; + + const { behavior = 'smooth', block = 'center' } = options; + try { + targetEl.scrollIntoView({ behavior, block, inline: 'nearest' }); + } catch { + // Ignore scroll failures in environments with incomplete DOM APIs. + return false; + } + return true; + } + + /** + * Scroll to the Nth heading of a given level (1..6). + * + * In OOXML headings are paragraphs whose `paragraphProperties.styleId` is + * `Heading1`..`Heading6` (the schema also accepts a `heading` node type + * with a `level` attr for editor-native callers). This walks the doc in + * order, counts headings whose level matches, and scrolls to the + * 1-based `ordinal`-th one. + * + * @param {number} level 1..6 + * @param {number} [ordinal=1] 1-based index among headings of that level + * @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition }} [options] + * @returns {Promise} Whether a matching heading was found and scrolled to + * + * @example + * // Scroll to the third top-level heading (a.k.a. chapter 3) + * await superdoc.scrollToHeading(1, 3); + */ + async scrollToHeading(level, ordinal = 1, options = {}) { + if (!Number.isInteger(level) || level < 1 || level > 6) return false; + if (!Number.isInteger(ordinal) || ordinal < 1) return false; + + const editor = this.activeEditor; + if (!editor?.state?.doc) return false; + + let count = 0; + let foundPos = null; + let foundNode = null; + editor.state.doc.descendants((node, p) => { + if (foundPos !== null) return false; + const t = node.type?.name; + let nodeLevel = null; + if (t === 'heading' && node.attrs?.level) { + nodeLevel = Number(node.attrs.level); + } else if (t === 'paragraph') { + const styleId = node.attrs?.paragraphProperties?.styleId ?? node.attrs?.styleId ?? null; + if (typeof styleId === 'string') { + const m = styleId.match(/^Heading(\d)$/); + if (m) nodeLevel = Number(m[1]); + } + } + if (nodeLevel === level) { + count += 1; + if (count === ordinal) { + foundPos = p; + foundNode = node; + return false; + } + } + }); + + if (foundPos === null) return false; + + // The position from descendants() is the doc-level boundary just + // BEFORE the heading paragraph. The presentation editor's + // layout-fragment index only covers positions INSIDE text content, + // so a doc-boundary position misses every fragment. Walk into the + // paragraph to find the first descendant that has actual text + // content (skipping bookmark markers, comment-range markers, etc.) + // and target that position instead. + if (foundNode && foundNode.content?.size > 0) { + let textInsidePos = null; + foundNode.descendants((child, offset) => { + if (textInsidePos !== null) return false; + if (child.isText && child.text && child.text.length > 0) { + // Position inside the paragraph = paragraph-start (foundPos+1) + // + descendant offset. + textInsidePos = foundPos + 1 + offset; + return false; + } + }); + if (textInsidePos != null) foundPos = textInsidePos; + } + + // Same dispatch as scrollToElement: presentation if available, else + // body-editor + DOM scrollIntoView. + const storeDocs = this.superdocStore?.documents; + const presentationEditor = storeDocs?.[0]?.getPresentationEditor?.(); + if (typeof presentationEditor?.scrollToPositionAsync === 'function') { + const ok = await presentationEditor.scrollToPositionAsync(foundPos, { + behavior: options.behavior ?? 'auto', + block: options.block ?? 'center', + }); + if (ok) return true; + // Fall through to body-editor path on layout-state miss. + } + + const targetEl = typeof editor.getElementAtPos === 'function' ? editor.getElementAtPos(foundPos) : null; + if (!targetEl?.scrollIntoView) return false; + + const { behavior = 'smooth', block = 'center' } = options; + try { + targetEl.scrollIntoView({ behavior, block, inline: 'nearest' }); + } catch { + return false; + } + return true; } /** From 5e66227bb5aa21c704f41edd21d2694ee7a07103 Mon Sep 17 00:00:00 2001 From: bjohas Date: Thu, 14 May 2026 09:07:48 +0100 Subject: [PATCH 3/6] test(superdoc): cover scrollToComment delegation + scrollToHeading Updates the two existing `scrollToComment` tests to reflect the new delegation-via-`scrollToElement` behaviour (no more bespoke `querySelector('[data-comment-ids*=...]')` lookup), and adds: - `scrollToElement` falls back to the body editor when the presentation path returns false (flow-layout case): uses `commands.setCursorById` to resolve the id and `getElementAtPos` for the DOM target. - `scrollToHeading` walks the doc, counts paragraphs whose `paragraphProperties.styleId = "HeadingN"`, picks the 1-based ordinal-th, and passes a text-inside-content position (not the doc-level boundary) to `scrollToPositionAsync`. - `scrollToHeading` rejects out-of-range levels (0, 7), non-positive ordinals, and non-integer arguments. All 84 `SuperDoc.test.js` cases pass. Co-Authored-By: Claude Opus 4.7 --- packages/superdoc/src/core/SuperDoc.test.js | 154 ++++++++++++++++++-- 1 file changed, 141 insertions(+), 13 deletions(-) diff --git a/packages/superdoc/src/core/SuperDoc.test.js b/packages/superdoc/src/core/SuperDoc.test.js index 70ab774045..b932db99b8 100644 --- a/packages/superdoc/src/core/SuperDoc.test.js +++ b/packages/superdoc/src/core/SuperDoc.test.js @@ -276,8 +276,15 @@ describe('SuperDoc core', () => { expect(instance.user).toEqual(expect.objectContaining({ name: 'Default SuperDoc user', email: null })); }); - it('scrolls to a comment and sets it active', async () => { - const { commentsStore } = createAppHarness(); + it('delegates scrollToComment to scrollToElement and marks the thread active', async () => { + const { superdocStore, commentsStore } = createAppHarness(); + const scrollToElement = vi.fn(async () => true); + superdocStore.documents = [ + { + getPresentationEditor: vi.fn(() => ({ scrollToElement })), + }, + ]; + const instance = new SuperDoc({ selector: '#host', document: 'https://example.com/doc.docx', @@ -289,19 +296,21 @@ describe('SuperDoc core', () => { }); await flushMicrotasks(); - const target = document.createElement('div'); - target.setAttribute('data-comment-ids', 'comment-1'); - target.scrollIntoView = vi.fn(); - document.querySelector('#host').appendChild(target); - - const result = instance.scrollToComment('comment-1'); - expect(result).toBe(true); - expect(target.scrollIntoView).toHaveBeenCalledWith({ behavior: 'smooth', block: 'start' }); + await expect(instance.scrollToComment('comment-1')).resolves.toBe(true); + expect(scrollToElement).toHaveBeenCalledWith('comment-1'); expect(commentsStore.setActiveComment).toHaveBeenCalledWith(instance, 'comment-1'); }); - it('returns false when comment element is not found', async () => { - createAppHarness(); + it('scrollToComment resolves false when neither presentation nor body editor can locate the id', async () => { + const { superdocStore } = createAppHarness(); + superdocStore.documents = [ + { + // Presentation editor present but returns false (e.g. mark on an unmounted page, + // or no such id in the doc). + getPresentationEditor: vi.fn(() => ({ scrollToElement: vi.fn(async () => false) })), + }, + ]; + const instance = new SuperDoc({ selector: '#host', document: 'https://example.com/doc.docx', @@ -311,7 +320,23 @@ describe('SuperDoc core', () => { }); await flushMicrotasks(); - expect(instance.scrollToComment('nonexistent-id')).toBe(false); + // No activeEditor on the instance → body-editor fallback also returns false. + await expect(instance.scrollToComment('nonexistent-id')).resolves.toBe(false); + }); + + it('scrollToComment returns false when comments module is disabled', async () => { + createAppHarness(); + const instance = new SuperDoc({ + selector: '#host', + document: 'https://example.com/doc.docx', + documents: [], + // no `modules.comments` + modules: { toolbar: {} }, + onException: vi.fn(), + }); + await flushMicrotasks(); + + await expect(instance.scrollToComment('any-id')).resolves.toBe(false); }); it('forwards navigateTo to the first presentation editor', async () => { @@ -407,6 +432,109 @@ describe('SuperDoc core', () => { await expect(instance.scrollToElement('element-1')).resolves.toBe(false); }); + it('scrollToElement falls back to body editor when presentation returns false', async () => { + const { superdocStore } = createAppHarness(); + superdocStore.documents = [ + { + // Presentation editor is present but cannot scroll (e.g. flow layout). + getPresentationEditor: vi.fn(() => ({ scrollToElement: vi.fn(async () => false) })), + }, + ]; + + const scrollIntoView = vi.fn(); + const targetEl = { scrollIntoView }; + const setCursorById = vi.fn(() => true); + + const instance = new SuperDoc({ + selector: '#host', + document: 'https://example.com/doc.docx', + documents: [], + modules: { comments: {}, toolbar: {} }, + onException: vi.fn(), + }); + await flushMicrotasks(); + + // Inject a minimal activeEditor stub. The body-editor fallback uses + // `setCursorById` to resolve the id and `getElementAtPos` to find the DOM. + Object.defineProperty(instance, 'activeEditor', { + configurable: true, + get: () => ({ + state: { + doc: { content: { size: 100 }, descendants: vi.fn() }, + selection: { from: 42 }, + }, + commands: { setCursorById }, + getElementAtPos: vi.fn(() => targetEl), + }), + }); + + await expect(instance.scrollToElement('comment-1')).resolves.toBe(true); + expect(setCursorById).toHaveBeenCalledWith('comment-1', { preferredActiveThreadId: 'comment-1' }); + expect(scrollIntoView).toHaveBeenCalledWith( + expect.objectContaining({ block: expect.any(String), inline: 'nearest' }), + ); + }); + + it('scrollToHeading walks for the Nth heading at the given level and scrolls', async () => { + const { superdocStore } = createAppHarness(); + // Mock doc with three Heading1 paragraphs at known positions. + const headings = [ + { pos: 10, text: 'first', styleId: 'Heading1' }, + { pos: 50, text: 'second', styleId: 'Heading1' }, + { pos: 200, text: 'third', styleId: 'Heading1' }, + ]; + const makeNode = (h) => ({ + type: { name: 'paragraph' }, + attrs: { paragraphProperties: { styleId: h.styleId } }, + content: { size: 5 }, + descendants: (cb) => cb({ isText: true, text: h.text }, 0), + }); + const descendants = (cb) => { + for (const h of headings) { + if (cb(makeNode(h), h.pos) === false) return; + } + }; + + const scrollToPositionAsync = vi.fn(async () => true); + superdocStore.documents = [{ getPresentationEditor: vi.fn(() => ({ scrollToPositionAsync })) }]; + + const instance = new SuperDoc({ + selector: '#host', + document: 'https://example.com/doc.docx', + documents: [], + modules: { comments: {}, toolbar: {} }, + onException: vi.fn(), + }); + await flushMicrotasks(); + + Object.defineProperty(instance, 'activeEditor', { + configurable: true, + get: () => ({ state: { doc: { descendants, content: { size: 1000 } } } }), + }); + + await expect(instance.scrollToHeading(1, 2)).resolves.toBe(true); + // The 2nd Heading1 starts at pos=50; the text-inside-content fix should + // shift the target one position into the paragraph's content. + expect(scrollToPositionAsync).toHaveBeenCalledWith(51, expect.any(Object)); + }); + + it('scrollToHeading rejects out-of-range levels and non-positive ordinals', async () => { + createAppHarness(); + const instance = new SuperDoc({ + selector: '#host', + document: 'https://example.com/doc.docx', + documents: [], + modules: { comments: {}, toolbar: {} }, + onException: vi.fn(), + }); + await flushMicrotasks(); + + await expect(instance.scrollToHeading(0, 1)).resolves.toBe(false); + await expect(instance.scrollToHeading(7, 1)).resolves.toBe(false); + await expect(instance.scrollToHeading(1, 0)).resolves.toBe(false); + await expect(instance.scrollToHeading(1.5, 1)).resolves.toBe(false); + }); + it('warns when both document object and documents list provided', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); createAppHarness(); From 4ded2ae81970c05af55dea586b98f99cbfd88a5f Mon Sep 17 00:00:00 2001 From: bjohas Date: Thu, 14 May 2026 16:06:32 +0100 Subject: [PATCH 4/6] feat(superdoc): scrollToHeading walks forward to next text content for empty headings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scrollToHeading targets a position INSIDE the heading paragraph's text content (not the doc-level boundary just before it) so the presentation editor's layout-fragment index — which only spans inside-text positions — has something to scroll to. The original patch did this by walking the heading node's own descendants. Refinement: when the heading itself carries no text content (empty paragraph, or content limited to structural markers like bookmarkStart / commentRangeStart), walk forward in the doc for the next text-bearing position instead of returning false. That way the viewport at least lands near where the empty heading lives — useful in user-facing TOC clicks where the click target is the empty heading entry the user intentionally selected. Co-Authored-By: Claude Opus 4.7 --- packages/superdoc/src/core/SuperDoc.js | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/superdoc/src/core/SuperDoc.js b/packages/superdoc/src/core/SuperDoc.js index a2c14469f6..c64fd91c06 100644 --- a/packages/superdoc/src/core/SuperDoc.js +++ b/packages/superdoc/src/core/SuperDoc.js @@ -1526,19 +1526,34 @@ export class SuperDoc extends EventEmitter { // paragraph to find the first descendant that has actual text // content (skipping bookmark markers, comment-range markers, etc.) // and target that position instead. + let resolved = null; if (foundNode && foundNode.content?.size > 0) { - let textInsidePos = null; foundNode.descendants((child, offset) => { - if (textInsidePos !== null) return false; + if (resolved !== null) return false; if (child.isText && child.text && child.text.length > 0) { // Position inside the paragraph = paragraph-start (foundPos+1) // + descendant offset. - textInsidePos = foundPos + 1 + offset; + resolved = foundPos + 1 + offset; return false; } }); - if (textInsidePos != null) foundPos = textInsidePos; } + if (resolved == null) { + // The heading itself carries no text content (truly-empty + // paragraph, or content limited to structural markers like + // bookmarkStart). Walk forward in the doc for the next text-bearing + // position so the viewport at least lands near where the heading + // lives instead of returning false. + editor.state.doc.descendants((child, p) => { + if (resolved !== null) return false; + if (p <= foundPos) return undefined; + if (child.isText && child.text && child.text.length > 0) { + resolved = p; + return false; + } + }); + } + if (resolved != null) foundPos = resolved; // Same dispatch as scrollToElement: presentation if available, else // body-editor + DOM scrollIntoView. From 1fcf8647866fa87a09be61f7cf4c6721e8be24cd Mon Sep 17 00:00:00 2001 From: bjohas Date: Thu, 14 May 2026 16:16:03 +0100 Subject: [PATCH 5/6] feat(super-editor): timeoutMs option on scrollToPositionAsync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scrollToPositionAsync` had a hard-coded 2 second timeout for waiting on the painter to mount the target virtualised page. Callers navigating long documents — where the painter may need longer than 2 s to settle when jumping far from the current viewport — had no way to extend it short of editing the static `ANCHOR_NAV_TIMEOUT_MS` constant. Adds an opt-in `timeoutMs` to the options bag. Backwards- compatible: defaults stay at 2000 ms when unset. The warn message now also includes the actual timeout that elapsed, which is useful when diagnosing whether a particular long-doc click was fighting the timeout or some other issue. `SuperDoc.scrollToHeading` passes the option through to `scrollToPositionAsync` when given, so external apps can extend the wait at the call site without reaching into the presentation editor directly: await superdoc.scrollToHeading(2, 18, { timeoutMs: 8000 }); `scrollToElement` / `navigateTo` deliberately don't yet thread the option — those traverse `#navigateToBlock` / `#navigateToComment` / `#navigateToTrackedChange` before reaching `scrollToPositionAsync`, which is a wider surface to touch. Can be added in a follow-up once the call sites that need it surface. Existing scrollToPosition test updated to match the new warn message; a new test exercises the option (short timeout fires faster than the default and the warn text includes the supplied value). 86/86 SuperDoc.test.js + 14/14 scrollToPosition.test.ts pass. Co-Authored-By: Claude Opus 4.7 --- .../presentation-editor/PresentationEditor.ts | 27 ++++++-- ...resentationEditor.scrollToPosition.test.ts | 29 +++++++- packages/superdoc/src/core/SuperDoc.js | 9 ++- packages/superdoc/src/core/SuperDoc.test.js | 67 +++++++++++++++++++ 4 files changed, 125 insertions(+), 7 deletions(-) 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 72ff565c6d..a6b66e62e9 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 @@ -3712,7 +3712,20 @@ export class PresentationEditor extends EventEmitter { */ async scrollToPositionAsync( pos: number, - options: { block?: 'start' | 'center' | 'end' | 'nearest'; behavior?: ScrollBehavior } = {}, + options: { + block?: 'start' | 'center' | 'end' | 'nearest'; + behavior?: ScrollBehavior; + /** + * Maximum time (ms) to wait for the painter to mount the target + * virtualised page before giving up. Defaults to + * `ANCHOR_NAV_TIMEOUT_MS` (2000 ms). Callers navigating across + * long documents — where the painter may need longer than 2 s to + * settle when jumping far from the current viewport — can extend + * this. The function still returns `false` on timeout, but the + * extension reduces false-negative anchor navigations. + */ + timeoutMs?: number; + } = {}, ): Promise { // Fast path: try sync scroll first (works if page already mounted) if (this.scrollToPosition(pos, options)) { @@ -3747,11 +3760,15 @@ export class PresentationEditor extends EventEmitter { this.#scrollPageIntoView(pageIndex); // Wait for page to mount in the DOM - const mounted = await this.#waitForPageMount(pageIndex, { - timeout: PresentationEditor.ANCHOR_NAV_TIMEOUT_MS, - }); + const timeout = + Number.isFinite(options.timeoutMs) && options.timeoutMs! > 0 + ? options.timeoutMs! + : PresentationEditor.ANCHOR_NAV_TIMEOUT_MS; + const mounted = await this.#waitForPageMount(pageIndex, { timeout }); if (!mounted) { - console.warn(`[PresentationEditor] scrollToPositionAsync: Page ${pageIndex} failed to mount within timeout`); + console.warn( + `[PresentationEditor] scrollToPositionAsync: Page ${pageIndex} failed to mount within ${timeout} ms`, + ); return false; } diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts index 76c3e7b0f3..1a3b9df88c 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.scrollToPosition.test.ts @@ -667,7 +667,34 @@ describe('PresentationEditor - scrollToPosition', () => { // The page never mounted, so it should fail expect(result).toBe(false); - expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('failed to mount within timeout')); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('failed to mount within')); + + consoleWarnSpy.mockRestore(); + }); + + it('honours a caller-supplied timeoutMs and surfaces the value in the warn message', 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)); + + // 100 ms ceiling — the page never mounts, so it should bail + // much faster than the 2 s default. + const t0 = Date.now(); + const result = await editor.scrollToPositionAsync(150, { timeoutMs: 100 }); + const elapsed = Date.now() - t0; + + expect(result).toBe(false); + // The warning should mention the supplied timeout (100 ms), not + // the static default. + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('100 ms')); + // And we should have bailed well under the 2 s default. + expect(elapsed).toBeLessThan(1500); consoleWarnSpy.mockRestore(); }); diff --git a/packages/superdoc/src/core/SuperDoc.js b/packages/superdoc/src/core/SuperDoc.js index c64fd91c06..34cdb48587 100644 --- a/packages/superdoc/src/core/SuperDoc.js +++ b/packages/superdoc/src/core/SuperDoc.js @@ -1477,7 +1477,11 @@ export class SuperDoc extends EventEmitter { * * @param {number} level 1..6 * @param {number} [ordinal=1] 1-based index among headings of that level - * @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition }} [options] + * @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition, timeoutMs?: number }} [options] + * Pass `timeoutMs` to override the default 2 s page-mount wait + * in paginated layout. Useful when jumping far from the current + * viewport on long docs where the painter takes longer to + * mount the target page. * @returns {Promise} Whether a matching heading was found and scrolled to * * @example @@ -1563,6 +1567,9 @@ export class SuperDoc extends EventEmitter { const ok = await presentationEditor.scrollToPositionAsync(foundPos, { behavior: options.behavior ?? 'auto', block: options.block ?? 'center', + // Pass-through so callers can extend the page-mount wait on + // long docs without reaching into PresentationEditor directly. + ...(Number.isFinite(options.timeoutMs) ? { timeoutMs: options.timeoutMs } : {}), }); if (ok) return true; // Fall through to body-editor path on layout-state miss. diff --git a/packages/superdoc/src/core/SuperDoc.test.js b/packages/superdoc/src/core/SuperDoc.test.js index b932db99b8..a9d83677c3 100644 --- a/packages/superdoc/src/core/SuperDoc.test.js +++ b/packages/superdoc/src/core/SuperDoc.test.js @@ -518,6 +518,73 @@ describe('SuperDoc core', () => { expect(scrollToPositionAsync).toHaveBeenCalledWith(51, expect.any(Object)); }); + it('scrollToHeading forwards an explicit timeoutMs to scrollToPositionAsync', async () => { + const { superdocStore } = createAppHarness(); + const headings = [{ pos: 10, text: 'only', styleId: 'Heading1' }]; + const makeNode = (h) => ({ + type: { name: 'paragraph' }, + attrs: { paragraphProperties: { styleId: h.styleId } }, + content: { size: 5 }, + descendants: (cb) => cb({ isText: true, text: h.text }, 0), + }); + const descendants = (cb) => { + for (const h of headings) { + if (cb(makeNode(h), h.pos) === false) return; + } + }; + const scrollToPositionAsync = vi.fn(async () => true); + superdocStore.documents = [{ getPresentationEditor: vi.fn(() => ({ scrollToPositionAsync })) }]; + const instance = new SuperDoc({ + selector: '#host', + document: 'https://example.com/doc.docx', + documents: [], + modules: { comments: {}, toolbar: {} }, + onException: vi.fn(), + }); + await flushMicrotasks(); + Object.defineProperty(instance, 'activeEditor', { + configurable: true, + get: () => ({ state: { doc: { descendants, content: { size: 1000 } } } }), + }); + + await expect(instance.scrollToHeading(1, 1, { timeoutMs: 10000 })).resolves.toBe(true); + expect(scrollToPositionAsync).toHaveBeenCalledWith(11, expect.objectContaining({ timeoutMs: 10000 })); + }); + + it('scrollToHeading omits timeoutMs when not given so the default applies', async () => { + const { superdocStore } = createAppHarness(); + const headings = [{ pos: 10, text: 'only', styleId: 'Heading1' }]; + const makeNode = (h) => ({ + type: { name: 'paragraph' }, + attrs: { paragraphProperties: { styleId: h.styleId } }, + content: { size: 5 }, + descendants: (cb) => cb({ isText: true, text: h.text }, 0), + }); + const descendants = (cb) => { + for (const h of headings) { + if (cb(makeNode(h), h.pos) === false) return; + } + }; + const scrollToPositionAsync = vi.fn(async () => true); + superdocStore.documents = [{ getPresentationEditor: vi.fn(() => ({ scrollToPositionAsync })) }]; + const instance = new SuperDoc({ + selector: '#host', + document: 'https://example.com/doc.docx', + documents: [], + modules: { comments: {}, toolbar: {} }, + onException: vi.fn(), + }); + await flushMicrotasks(); + Object.defineProperty(instance, 'activeEditor', { + configurable: true, + get: () => ({ state: { doc: { descendants, content: { size: 1000 } } } }), + }); + + await expect(instance.scrollToHeading(1, 1)).resolves.toBe(true); + const opts = scrollToPositionAsync.mock.calls[0][1]; + expect(opts.timeoutMs).toBeUndefined(); + }); + it('scrollToHeading rejects out-of-range levels and non-positive ordinals', async () => { createAppHarness(); const instance = new SuperDoc({ From 0bbe546d577ab843ca28309f1595b25d27f409de Mon Sep 17 00:00:00 2001 From: bjohas Date: Thu, 14 May 2026 17:21:57 +0100 Subject: [PATCH 6/6] fix(superdoc): scrollToElement matches any of nodeId/sdBlockId/id/paraId per node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Block nodes in imported docx files can carry more than one ID-shaped attr at once — paragraphs from `.docx` carry both `paraId` (from `w14:paraId` in OOXML) and `sdBlockId` (minted by SuperDoc on import). The previous walk did `const candidate = a.nodeId ?? a.sdBlockId ?? a.id ?? a.paraId` and compared the single first-non-null pick to the caller's elementId. That meant a paragraph carrying both `sdBlockId: "3496bf7f-..."` and `paraId: "00000001"` would always return `sdBlockId` as the candidate and never match a caller looking for `"00000001"`. Compare each attr independently so the walk matches when ANY of the candidate fields equals elementId. Consumers can now pass whichever ID handle they have without knowing which ID types a given block happens to carry. Adds one regression test asserting that scrollToElement('00000001') resolves to `true` even when the matching paragraph also has a non-matching sdBlockId. Co-Authored-By: Claude Opus 4.7 --- packages/superdoc/src/core/SuperDoc.js | 11 +++-- packages/superdoc/src/core/SuperDoc.test.js | 45 +++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/packages/superdoc/src/core/SuperDoc.js b/packages/superdoc/src/core/SuperDoc.js index 34cdb48587..c9f7af0227 100644 --- a/packages/superdoc/src/core/SuperDoc.js +++ b/packages/superdoc/src/core/SuperDoc.js @@ -1438,13 +1438,18 @@ export class SuperDoc extends EventEmitter { } // 2. Fall back to a single PM walk looking for matching block-level - // id attributes (nodeId / sdBlockId / id / paraId). + // id attributes. Block nodes can carry multiple ID-shaped attrs + // at once — e.g. paragraphs from a `.docx` carry both `paraId` + // (the OOXML `w14:paraId`) and `sdBlockId` (minted by SuperDoc + // on import). We must compare against each independently rather + // than picking the first non-null and comparing, because the + // caller may have a handle on any one of them and consumers + // shouldn't have to know which ID type a given block carries. if (pos == null || !Number.isFinite(pos)) { editor.state.doc.descendants((node, p) => { if (pos != null) return false; const a = node.attrs || {}; - const candidate = a.nodeId ?? a.sdBlockId ?? a.id ?? a.paraId; - if (candidate && candidate === elementId) { + if (a.nodeId === elementId || a.sdBlockId === elementId || a.id === elementId || a.paraId === elementId) { pos = p; return false; } diff --git a/packages/superdoc/src/core/SuperDoc.test.js b/packages/superdoc/src/core/SuperDoc.test.js index a9d83677c3..e359c48a97 100644 --- a/packages/superdoc/src/core/SuperDoc.test.js +++ b/packages/superdoc/src/core/SuperDoc.test.js @@ -475,6 +475,51 @@ describe('SuperDoc core', () => { ); }); + it('scrollToElement matches paraId even when sdBlockId is also present', async () => { + const { superdocStore } = createAppHarness(); + superdocStore.documents = [{ getPresentationEditor: vi.fn(() => ({ scrollToElement: vi.fn(async () => false) })) }]; + + const scrollIntoView = vi.fn(); + const targetEl = { scrollIntoView }; + const setCursorById = vi.fn(() => false); + + // A paragraph carrying BOTH a long sdBlockId and a short paraId. + // The walk must match against each attr independently — picking + // the first non-null and comparing would let sdBlockId mask paraId. + const node = { + attrs: { + sdBlockId: '3496bf7f-b408-489d-9d1d-7a6854c09e70', + paraId: '00000001', + }, + }; + const descendants = (cb) => { + cb(node, 7); + }; + + const instance = new SuperDoc({ + selector: '#host', + document: 'https://example.com/doc.docx', + documents: [], + modules: { comments: {}, toolbar: {} }, + onException: vi.fn(), + }); + await flushMicrotasks(); + + Object.defineProperty(instance, 'activeEditor', { + configurable: true, + get: () => ({ + state: { doc: { descendants, content: { size: 100 } }, selection: { from: null } }, + commands: { setCursorById }, + getElementAtPos: vi.fn(() => targetEl), + }), + }); + + await expect(instance.scrollToElement('00000001')).resolves.toBe(true); + expect(scrollIntoView).toHaveBeenCalledWith( + expect.objectContaining({ block: expect.any(String), inline: 'nearest' }), + ); + }); + it('scrollToHeading walks for the Nth heading at the given level and scrolls', async () => { const { superdocStore } = createAppHarness(); // Mock doc with three Heading1 paragraphs at known positions.