feat(superdoc): scrollToHeading API + fix scrollToComment / scrollToElement in flow layout#3276
Draft
bjohas wants to merge 6 commits into
Draft
feat(superdoc): scrollToHeading API + fix scrollToComment / scrollToElement in flow layout#3276bjohas wants to merge 6 commits into
bjohas wants to merge 6 commits into
Conversation
`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 <noreply@anthropic.com>
…ading
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>
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 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…r empty headings 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 <noreply@anthropic.com>
`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 <noreply@anthropic.com>
This was referenced May 14, 2026
…aId per node
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related changes to the public scroll-to-X surface, motivated by an external integration (Electron-based DOCX viewer) where comment-panel click → editor scroll was silently broken in flow layout and unreliable in paginated layout.
fix(super-editor)—Editor.getElementAtPosin flow / web layout now resolvesview.domAtPos's parent-element + offset to the actual child element, instead of returning the parent. Previously, every position in the doc collapsed to "the entire editor root," soscrollIntoViewalways landed at the top.fix(superdoc)—SuperDoc.scrollToComment(id)now delegates toscrollToElement(id). The oldroot.querySelector('[data-comment-ids*="..."]')lookup missed virtualised pages in paginated mode and missed marks for which SuperDoc hadn't emitted a visible highlight span.fix(superdoc) + feat(superdoc)—SuperDoc.scrollToElement(id)now falls back to a body-editor path when no presentation editor exists (flow layout) or when presentation returns false. Walks the PM doc viacommands.setCursorByIdfor comment / tracked-change marks, with an attr-match fallback for block IDs.feat(superdoc)— newSuperDoc.scrollToHeading(level, ordinal, options)public API. Walks the doc counting paragraphs whoseparagraphProperties.styleId = "HeadingN"(orheadingnodes with alevelattr), targets a position inside the heading's text content (not the doc-level boundary before the paragraph — the layout-fragment index doesn't cover boundaries), and dispatches through the same flow/paginated split asscrollToElement.Motivation
In an external app embedding SuperDoc 1.31.2:
superdoc.scrollToComment(id)returnedtruebut didn't actually scroll in many cases (flow + paginated mismatches between marks and highlight spans).superdoc.scrollToElement(id)returnedfalsefor every comment in flow layout (no presentation editor → unconditional bail-out).With this PR, against a small commented test docx:
scrollToCommentflowscrollToCommentpaginatedscrollToPositionAsyncedge case)scrollToHeading(level, ordinal)flowscrollToHeading(level, ordinal)paginatedThe one residual paginated case (a comment whose marks land at pos=13240/42/47) is
scrollToPositionAsyncreturningtruewithout moving the scroll — out of scope for this PR; happy to file separately.Test plan
pnpm --filter superdoc test src/core/SuperDoc.test.js— 84/84 pass, including 4 new cases covering the new delegation, body-editor fallback, and heading walk.pnpm --filter @superdoc/super-editor test src/editors/v1/core/Editor— 160/160 pass (no regression aroundgetElementAtPos).pnpm --filter @superdoc/super-editor test src/editors/v1/core/presentation-editor— 1369/1369 pass.dist/against the 4-comment / 41-heading test docx. Numbers above.Notes
tests/consumer-typecheck/per CONTRIBUTING if you'd prefer the newscrollToHeadingsignature pinned there before merge.Editor.getElementAtPoschange is the most cross-cutting — only touches the body-editor branch (presentation-editor implementation is unchanged) and internal callers all use.closest(...)to climb to the class they want, which still works with the more specific child element this now returns.🤖 Generated with Claude Code