Skip to content

Commit e44c627

Browse files
bjohasclaude
andcommitted
feat(superdoc): fix scrollToComment + scrollToElement, add scrollToHeading
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 <noreply@anthropic.com>
1 parent 6de21f6 commit e44c627

1 file changed

Lines changed: 191 additions & 16 deletions

File tree

packages/superdoc/src/core/SuperDoc.js

Lines changed: 191 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,27 +1323,29 @@ export class SuperDoc extends EventEmitter {
13231323
/**
13241324
* Scroll the document to a given comment by id.
13251325
*
1326+
* Delegates to {@link scrollToElement} so it works in both flow and
1327+
* paginated layouts. The previous implementation looked up the highlight
1328+
* span via `[data-comment-ids*=...]` and called `scrollIntoView()` on it
1329+
* directly — that fails in paginated/print mode (the painter virtualises
1330+
* pages so the highlight may not be in the DOM) and also fails for marks
1331+
* SuperDoc didn't emit a visible highlight for (e.g. two marks sharing a
1332+
* single position). The unified path walks the ProseMirror doc for the
1333+
* mark and dispatches to the presentation editor where available,
1334+
* falling back to the body editor in flow mode.
1335+
*
13261336
* @param {string} commentId The comment id
13271337
* @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition }} [options]
1328-
* @returns {boolean} Whether a matching element was found
1338+
* @returns {Promise<boolean>} Whether the comment was found and scrolled to
13291339
*/
1330-
scrollToComment(commentId, options = {}) {
1340+
async scrollToComment(commentId, options = {}) {
13311341
const commentsConfig = this.config?.modules?.comments;
1332-
// `commentsConfig` can be `false | object | undefined`; `!commentsConfig`
1333-
// already covers both `false` and `undefined`, so the secondary
1334-
// `=== false` check below is redundant.
13351342
if (!commentsConfig) return false;
13361343
if (!commentId || typeof commentId !== 'string') return false;
13371344

1338-
const root = this.element || document;
1339-
const escaped = globalThis.CSS?.escape ? globalThis.CSS.escape(commentId) : commentId.replace(/"/g, '\\"');
1340-
const element = root.querySelector(`[data-comment-ids*="${escaped}"]`);
1341-
if (!element) return false;
1342-
1343-
const { behavior = 'smooth', block = 'start' } = options ?? {};
1344-
element.scrollIntoView({ behavior, block });
1345+
// Activate the thread in the side panel for visual continuity even if
1346+
// the scroll path subsequently bails.
13451347
this.commentsStore?.setActiveComment?.(this, commentId);
1346-
return true;
1348+
return this.scrollToElement(commentId, options);
13471349
}
13481350

13491351
/**
@@ -1371,7 +1373,13 @@ export class SuperDoc extends EventEmitter {
13711373
* change entityId. The method resolves the element type automatically
13721374
* and scrolls to it.
13731375
*
1376+
* In paginated (presentation) layouts this delegates to the
1377+
* presentation editor's `scrollToElement`. In flow / web layouts the
1378+
* presentation editor doesn't apply, so we fall back to walking the
1379+
* ProseMirror doc directly and scrolling the body editor's view.
1380+
*
13741381
* @param {string} elementId - The element's stable ID.
1382+
* @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition }} [options]
13751383
* @returns {Promise<boolean>} Whether the element was found and scrolled to.
13761384
*
13771385
* @example
@@ -1381,13 +1389,180 @@ export class SuperDoc extends EventEmitter {
13811389
* // Navigate to a comment by its entityId
13821390
* await superdoc.scrollToElement('imported-25def254');
13831391
*/
1384-
async scrollToElement(elementId) {
1392+
async scrollToElement(elementId, options = {}) {
1393+
if (!elementId) return false;
13851394
/** @type {RuntimeDocument[] | undefined} */
13861395
const storeDocs = this.superdocStore?.documents;
13871396
if (!storeDocs?.length) return false;
1397+
13881398
const presentationEditor = storeDocs[0].getPresentationEditor?.();
1389-
if (!presentationEditor?.scrollToElement) return false;
1390-
return presentationEditor.scrollToElement(elementId);
1399+
if (presentationEditor?.scrollToElement) {
1400+
const ok = await presentationEditor.scrollToElement(elementId);
1401+
if (ok) return true;
1402+
// Otherwise: presentationEditor may have returned false because layout
1403+
// state isn't active (flow/web mode masquerading as presentation). Fall
1404+
// through to the body-editor path.
1405+
}
1406+
1407+
return this._scrollToElementInBodyEditor(elementId, options);
1408+
}
1409+
1410+
/**
1411+
* Flow-layout fallback for {@link scrollToElement}.
1412+
*
1413+
* The body editor IS the visible view in flow mode, so we walk PM for the
1414+
* target position and use the editor's own DOM-by-position helper, then
1415+
* scroll the resulting element into view. Tries comment / tracked-change
1416+
* marks (via the existing `setCursorById` command) first, then falls back
1417+
* to block-level node IDs (paragraphs, headings, tables) by attribute
1418+
* matching.
1419+
*
1420+
* @param {string} elementId
1421+
* @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition }} [options]
1422+
* @returns {Promise<boolean>}
1423+
* @private
1424+
*/
1425+
async _scrollToElementInBodyEditor(elementId, options = {}) {
1426+
const editor = this.activeEditor;
1427+
if (!editor?.state?.doc) return false;
1428+
1429+
let pos = null;
1430+
1431+
// 1. Try the comments-plugin command — handles commentMark, tracked
1432+
// change marks, and commentRangeStart/End nodes uniformly.
1433+
const setCursorById = editor.commands?.setCursorById;
1434+
if (typeof setCursorById === 'function') {
1435+
if (setCursorById(elementId, { preferredActiveThreadId: elementId })) {
1436+
pos = editor.state.selection?.from;
1437+
}
1438+
}
1439+
1440+
// 2. Fall back to a single PM walk looking for matching block-level
1441+
// id attributes (nodeId / sdBlockId / id / paraId).
1442+
if (pos == null || !Number.isFinite(pos)) {
1443+
editor.state.doc.descendants((node, p) => {
1444+
if (pos != null) return false;
1445+
const a = node.attrs || {};
1446+
const candidate = a.nodeId ?? a.sdBlockId ?? a.id ?? a.paraId;
1447+
if (candidate && candidate === elementId) {
1448+
pos = p;
1449+
return false;
1450+
}
1451+
});
1452+
}
1453+
1454+
if (pos == null || !Number.isFinite(pos)) return false;
1455+
1456+
const targetEl = typeof editor.getElementAtPos === 'function' ? editor.getElementAtPos(pos) : null;
1457+
if (!targetEl?.scrollIntoView) return false;
1458+
1459+
const { behavior = 'smooth', block = 'center' } = options;
1460+
try {
1461+
targetEl.scrollIntoView({ behavior, block, inline: 'nearest' });
1462+
} catch {
1463+
// Ignore scroll failures in environments with incomplete DOM APIs.
1464+
return false;
1465+
}
1466+
return true;
1467+
}
1468+
1469+
/**
1470+
* Scroll to the Nth heading of a given level (1..6).
1471+
*
1472+
* In OOXML headings are paragraphs whose `paragraphProperties.styleId` is
1473+
* `Heading1`..`Heading6` (the schema also accepts a `heading` node type
1474+
* with a `level` attr for editor-native callers). This walks the doc in
1475+
* order, counts headings whose level matches, and scrolls to the
1476+
* 1-based `ordinal`-th one.
1477+
*
1478+
* @param {number} level 1..6
1479+
* @param {number} [ordinal=1] 1-based index among headings of that level
1480+
* @param {{ behavior?: ScrollBehavior, block?: ScrollLogicalPosition }} [options]
1481+
* @returns {Promise<boolean>} Whether a matching heading was found and scrolled to
1482+
*
1483+
* @example
1484+
* // Scroll to the third top-level heading (a.k.a. chapter 3)
1485+
* await superdoc.scrollToHeading(1, 3);
1486+
*/
1487+
async scrollToHeading(level, ordinal = 1, options = {}) {
1488+
if (!Number.isInteger(level) || level < 1 || level > 6) return false;
1489+
if (!Number.isInteger(ordinal) || ordinal < 1) return false;
1490+
1491+
const editor = this.activeEditor;
1492+
if (!editor?.state?.doc) return false;
1493+
1494+
let count = 0;
1495+
let foundPos = null;
1496+
let foundNode = null;
1497+
editor.state.doc.descendants((node, p) => {
1498+
if (foundPos !== null) return false;
1499+
const t = node.type?.name;
1500+
let nodeLevel = null;
1501+
if (t === 'heading' && node.attrs?.level) {
1502+
nodeLevel = Number(node.attrs.level);
1503+
} else if (t === 'paragraph') {
1504+
const styleId = node.attrs?.paragraphProperties?.styleId ?? node.attrs?.styleId ?? null;
1505+
if (typeof styleId === 'string') {
1506+
const m = styleId.match(/^Heading(\d)$/);
1507+
if (m) nodeLevel = Number(m[1]);
1508+
}
1509+
}
1510+
if (nodeLevel === level) {
1511+
count += 1;
1512+
if (count === ordinal) {
1513+
foundPos = p;
1514+
foundNode = node;
1515+
return false;
1516+
}
1517+
}
1518+
});
1519+
1520+
if (foundPos === null) return false;
1521+
1522+
// The position from descendants() is the doc-level boundary just
1523+
// BEFORE the heading paragraph. The presentation editor's
1524+
// layout-fragment index only covers positions INSIDE text content,
1525+
// so a doc-boundary position misses every fragment. Walk into the
1526+
// paragraph to find the first descendant that has actual text
1527+
// content (skipping bookmark markers, comment-range markers, etc.)
1528+
// and target that position instead.
1529+
if (foundNode && foundNode.content?.size > 0) {
1530+
let textInsidePos = null;
1531+
foundNode.descendants((child, offset) => {
1532+
if (textInsidePos !== null) return false;
1533+
if (child.isText && child.text && child.text.length > 0) {
1534+
// Position inside the paragraph = paragraph-start (foundPos+1)
1535+
// + descendant offset.
1536+
textInsidePos = foundPos + 1 + offset;
1537+
return false;
1538+
}
1539+
});
1540+
if (textInsidePos != null) foundPos = textInsidePos;
1541+
}
1542+
1543+
// Same dispatch as scrollToElement: presentation if available, else
1544+
// body-editor + DOM scrollIntoView.
1545+
const storeDocs = this.superdocStore?.documents;
1546+
const presentationEditor = storeDocs?.[0]?.getPresentationEditor?.();
1547+
if (typeof presentationEditor?.scrollToPositionAsync === 'function') {
1548+
const ok = await presentationEditor.scrollToPositionAsync(foundPos, {
1549+
behavior: options.behavior ?? 'auto',
1550+
block: options.block ?? 'center',
1551+
});
1552+
if (ok) return true;
1553+
// Fall through to body-editor path on layout-state miss.
1554+
}
1555+
1556+
const targetEl = typeof editor.getElementAtPos === 'function' ? editor.getElementAtPos(foundPos) : null;
1557+
if (!targetEl?.scrollIntoView) return false;
1558+
1559+
const { behavior = 'smooth', block = 'center' } = options;
1560+
try {
1561+
targetEl.scrollIntoView({ behavior, block, inline: 'nearest' });
1562+
} catch {
1563+
return false;
1564+
}
1565+
return true;
13911566
}
13921567

13931568
/**

0 commit comments

Comments
 (0)