Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions packages/super-editor/src/editors/v1/core/Editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1718,11 +1718,30 @@ export class Editor extends EventEmitter<EditorEventMap> {
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: <text>, offset: <chars into text> }, or
// - { node: <element>, offset: <child index> } 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean> {
// Fast path: try sync scroll first (works if page already mounted)
if (this.scrollToPosition(pos, options)) {
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
234 changes: 218 additions & 16 deletions packages/superdoc/src/core/SuperDoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean>} 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);
}

/**
Expand Down Expand Up @@ -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<boolean>} Whether the element was found and scrolled to.
*
* @example
Expand All @@ -1381,13 +1389,207 @@ 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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Forward scroll options in paginated mode

When scrollToElement() or the new scrollToComment(..., options) path succeeds through a presentation editor, the caller's behavior/block options are dropped here, so paginated layout always uses the presentation defaults instead of the public options documented on SuperDoc. This regresses scrollToComment(id, { block: 'start', behavior: 'smooth' }) for comments rendered in paginated mode.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: options (behavior/block) are accepted by the public API but never forwarded to presentationEditor.scrollToElement(elementId). In paginated mode, callers passing { block: 'start', behavior: 'smooth' } will always get the presentation editor's internal defaults instead. The body-editor fallback correctly uses options, and scrollToHeading correctly passes options to scrollToPositionAsync, so this path is inconsistent.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/superdoc/src/core/SuperDoc.js, line 1400:

<comment>`options` (behavior/block) are accepted by the public API but never forwarded to `presentationEditor.scrollToElement(elementId)`. In paginated mode, callers passing `{ block: 'start', behavior: 'smooth' }` will always get the presentation editor's internal defaults instead. The body-editor fallback correctly uses `options`, and `scrollToHeading` correctly passes options to `scrollToPositionAsync`, so this path is inconsistent.</comment>

<file context>
@@ -1381,13 +1389,207 @@ export class SuperDoc extends EventEmitter {
-    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
</file context>
Suggested change
const ok = await presentationEditor.scrollToElement(elementId);
const ok = await presentationEditor.scrollToElement(elementId, options);
Fix with Cubic

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<boolean>}
* @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. 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 || {};
if (a.nodeId === elementId || a.sdBlockId === elementId || a.id === elementId || a.paraId === 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, 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<boolean>} 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)$/);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Recognize existing heading styleId variants

When imported or edited documents store heading styles as variants like heading 2 or HEADING 4, this new API returns false because it only matches the exact HeadingN form. The existing document-address resolver already treats /heading\s*([1-6])/i as a heading, so scrollToHeading() will disagree with the rest of the document API for those valid heading paragraphs.

Useful? React with 👍 / 👎.

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.
let resolved = null;
if (foundNode && foundNode.content?.size > 0) {
foundNode.descendants((child, offset) => {
if (resolved !== null) return false;
if (child.isText && child.text && child.text.length > 0) {
// Position inside the paragraph = paragraph-start (foundPos+1)
// + descendant offset.
resolved = foundPos + 1 + offset;
return false;
}
});
}
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.
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',
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Inconsistent default behavior between the presentation-editor path ('auto') and the body-editor fallback ('smooth'). Callers that omit options.behavior will see an animated scroll in flow layout but an instant jump in paginated layout.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/superdoc/src/core/SuperDoc.js, line 1573:

<comment>Inconsistent default `behavior` between the presentation-editor path (`'auto'`) and the body-editor fallback (`'smooth'`). Callers that omit `options.behavior` will see an animated scroll in flow layout but an instant jump in paginated layout.</comment>

<file context>
@@ -1381,13 +1389,207 @@ export class SuperDoc extends EventEmitter {
+    const presentationEditor = storeDocs?.[0]?.getPresentationEditor?.();
+    if (typeof presentationEditor?.scrollToPositionAsync === 'function') {
+      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
</file context>
Fix with Cubic

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.
}

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;
}

/**
Expand Down
Loading
Loading