feat(ui): scroll a content control into view by id (SD-3310)#3562
Merged
Conversation
Scroll a content control into view by id. The position is resolved from the document model, not the painted DOM, so it works for off-screen controls - the page mounts, then scrolls (mirrors the comment / tracked-change navigate path). Scroll-only: focus/activate stays a separate concern. Match the id with String() normalization so a control whose PM attr is numeric still resolves from a consumer's string id (the list / painted data-sdt-id are always strings). Kept UI-local rather than widening the Document API address union, matching how getRect resolves a content control. Updates the custom-UI docs and drops the now-stale "no scroll-to-control" limit.
Contributor
|
📖 Docs preview: https://superdoc-caio-sd-3310-scrollintoview.mintlify.app |
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
No issues found across 6 files
Linked issue analysis
Linked issue: SD-3310: feat(ui): ui.contentControls.scrollIntoView({ id, block?, behavior? })
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Expose ui.contentControls.scrollIntoView({ id, block?, behavior? }) on the ContentControls handle | The UI handle and public types add the scrollIntoView method and document its shape. |
| ✅ | Resolve the control position from the document model so it works when the target is off-screen / not yet rendered (mount page, then scroll) | PresentationEditor.scrollContentControlIntoView walks the model to a text position and calls scrollToPositionAsync (which mounts then scrolls); Playwright behavior tests verify off-screen inline and block controls are brought into view. |
| ✅ | v1 is body-only: controls inside header/footer/note stories return { success: false } | The PresentationEditor method and the UI handle explicitly document and implement a body-only path; the code returns false when the presentation/editor path can't resolve a body control. |
| ✅ | Normalize id comparison so a numeric PM node id attr still resolves when passed as a string | The implementation compares String(node.attrs?.id) to the supplied id and the behavior tests include a numeric-id case that succeeds. |
| ✅ | Default options: block defaults to 'center' and behavior to 'smooth'; explicit options pass through; return maps boolean to { success } | create-super-doc-ui provides center/smooth defaults and maps the presentation boolean result to { success }; unit tests assert defaults and explicit option pass-through and mapping. |
| Scroll-only: does not move selection/caret into the control (focus/activate is out of scope) | The code and comments explicitly state this is scroll-only and the implementation calls scrollToPositionAsync rather than changing selection, but there is no unit/behavior test that asserts the selection/caret was not moved as part of this PR. |
Tip: cubic could auto-approve low-risk PRs like this, if it thinks it's safe to merge. Learn more
Re-trigger cubic
Load the NDA template fixture (13 real Word content controls) and scroll its first inline field and last block clause into view from off-screen. Guards the imported-docx path; the other specs only used programmatically-created controls. Fixture copied into the suite to avoid coupling to demos/.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
|
🎉 This PR is included in superdoc-cli v0.15.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-sdk v1.14.0 |
Contributor
|
🎉 This PR is included in @superdoc-dev/mcp v0.10.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc v1.38.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in @superdoc-dev/react v1.9.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in vscode-ext v2.10.0 |
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
2 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.
Custom field and clause UIs (sidebars, navigators) need to jump to a control, but there was no public scroll-to.
getRecttells you where a control is; this adds the matching "scroll it there".It resolves the control's position from the document model rather than the painted DOM, so it works for off-screen controls the same way comment and tracked-change navigation already do: find the page, mount it, scroll. Scroll-only on purpose - moving the cursor into a control is a separate, trickier decision (inline vs block, locked, suggesting mode) kept out of scope.
Kept it UI-local instead of adding a content-control address to the Document API navigate union. That mirrors how
getRectalready resolves controls and avoids growing the contract surface for a viewport concern.ui.contentControls.scrollIntoView({ id, block?, behavior? }), returns{ success }{ success: false }String()normalization so a numeric PM attr still resolves from a consumer's string idReview: the model-position resolution in
PresentationEditor.scrollContentControlIntoView(descendants walk to the first text node, thenscrollToPositionAsync). The behavior test forces an off-screen start by scrolling; the virtualized-page mount itself isscrollToPositionAsync's own tested path.Verified: content-controls.test.ts 16/16; behavior content-control-scroll-into-view 4/4 (chromium); pnpm check:types clean; pnpm check:public:superdoc 13/13