feat(ui): focus a content control to place the caret inside it (SD-3312)#3571
Merged
Conversation
…l (SD-3312)
focus({ id, block?, behavior? }) scrolls a content control into view and places
the caret inside it - the "take me there and let me edit" counterpart to the
scroll-only scrollIntoView. Caret-inside (both SDT node types are atom:false, so
a TextSelection inside is the meaningful selection). Selection, not mutation: it
does not bypass lock or document-mode rules, so a locked / read-only control can
be focused for inspection but edits stay blocked. Returns { success } or
{ success: false, reason } only for real navigation problems (invalid-id /
not-ready / not-found); v1 is body-only.
Reuses the scroll method's node resolution. contract-templates dogfoods it with
a Focus button beside Locate on field rows and clause cards; adds a demo
acceptance test (Focus lands the caret inside the control) and handle unit tests.
Documents it and drops the now-resolved "no focus-by-id" limit.
Contributor
|
📖 Docs preview: https://superdoc-caio-sd-3312-focus-activate.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
2 issues found across 8 files
Linked issue analysis
Linked issue: SD-3312: spike: ui.contentControls.focus({ id }) / activate({ id })
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Add a public ui.contentControls.focus({ id, block?, behavior? }) API and update public types/docs. | The UI handle, public types, and docs are added/updated to expose focus(). create-super-doc-ui.ts and types.ts implement the API surface; content-controls.mdx documents it. |
| ✅ | Focus scrolls the control into view first and then places the caret inside the control (caret-inside selection). | PresentationEditor.focusContentControl resolves a caret position, calls scrollToPositionAsync, then sets a text selection at that position. The demo wiring and Playwright tests verify caret ends up inside the target control. |
| ✅ | Honor the scroll result and fail with not-reachable if the control can't be brought into view; return explicit failure reasons for navigation errors. | focusContentControl scrolls first and returns { success: false, reason: 'not-reachable' } when scrolling fails; types enumerate the allowable failure reasons and unit tests cover invalid-id / not-ready behavior while the demo covers the off-screen path. |
| ✅ | Default options and option passthrough: block defaults to 'center' and behavior to 'smooth'; explicit options pass through. | create-super-doc-ui.focus supplies block:'center' and behavior:'smooth' defaults; unit test asserts the presentation method is called with those defaults and that explicit options are forwarded. |
| Focus does not bypass locks / read-only (selection not mutation). | The docs and comments state focus is selection-only and should not bypass lock or document-mode rules. The implementation places a caret (setTextSelection) and the public surface documents the intent, but there are no explicit unit tests exercising locked/read-only behavior to fully verify enforcement. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…-3312)
focus now fails with not-ready when setTextSelection is unavailable and
not-reachable when it doesn't place the caret, so { success: true } means the
caret was actually placed (was reported regardless via optional chaining).
Docs: introduce the smartField tag convention inline instead of referencing a
"convention above" that wasn't introduced earlier on the page.
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.
Completes the custom content-control UI loop:
scrollIntoViewshows a control;focustakes you there AND puts the caret inside so the user can type.It's selection, not mutation - it does not bypass lock or document-mode rules, so a locked or read-only control can be focused for inspection while edits stay blocked by the normal rules. Caret-inside, because both SDT node types are
atom: false(a wrapper NodeSelection isn't the meaningful selection). It scrolls first and honors the result: a focus that can't bring the control into view returns{ success: false, reason: 'not-reachable' }rather than leaving a caret on a page that never mounted. Other failures are real navigation problems only (invalid-id/not-ready/not-found); v1 is body-only.ui.contentControls.focus({ id, block?, behavior? })→{ success }|{ success: false, reason }focus, drops the now-resolved "no focus-by-id" limit, adds a short "how the model works" note (overlay model; custom types viatag; round-trips to.docx)Review: the scroll-before-selection ordering in
focusContentControl(honors the scroll result, mirrors#scrollToBlockCandidate). The off-screen-clause demo test covers the hard path, not just a visible field.Verified: content-controls units 19/19; demo suite 5/5 incl off-screen focus (scrolls in + caret inside);
pnpm check:typesclean;pnpm check:public:superdoc13/13