fix(sdt): correct cursor placement and label interactions for structured content (SD-3237)#3444
Merged
caio-pizzol merged 154 commits intoMay 28, 2026
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
luccas-harbour
force-pushed
the
luccas/sd-3237-bug-sdt-hover-and-click-to-place-cursor-interactions-are
branch
from
6da06be to
d08a48d
Compare
caio-pizzol
reviewed
May 26, 2026
Contributor
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
the inline label path looks good. two label-click issues to fix before merge - left both inline.
| editor.view?.dispatch(tr); | ||
| } | ||
|
|
||
| this.#focusEditor(); |
Contributor
There was a problem hiding this comment.
clicking a block SDT label selects the block and focuses the editor, but it never calls scheduleSelectionUpdate. That leaves selection listeners out of date until something else updates them. The inline label path already does this.
Suggested change
| this.#focusEditor(); | |
| this.#callbacks.scheduleSelectionUpdate?.(); | |
| this.#focusEditor(); |
…red content - Restore normal caret placement inside block and inline SDTs; remove SD-1584 whole-node selection on body click and the first-click select-all behavior on inline SDT content. - Rewrite label click handling in EditorInputManager: move pointerdown to the visible host with capture, fall back to elementsFromPoint to resolve hover affordances, and apply NodeSelection (with pm-start retry) on label hits. - Exclude `superdoc-structured-content__label` and inline-label chrome from layout/dom pointer mapping so label rects don't bleed into body caret resolution. - Drop sdt-group-hover style that revealed the block label without selection. - Add SD-3237 behavior spec and fixture covering hover, click-to-place-cursor, and label-selection across nested SDTs; refresh related unit tests.
Selecting a structured content label on pointerdown was swallowing the drag handle's native dragstart, so labels could be clicked but not dragged. Move the click-to-select handling to pointerup/click with a movement threshold, and listen on the visible host, viewport host, and owner document so the event is caught regardless of which surface ProseMirror reattaches the label to. Also stop the structured-content select plugin from collapsing a parent block selection that merely contains a nested inline SDT, and update the cursor-placement tests to match the new flow.
…ng run (SD-3165) Adds a new selectInlineSdtBeforeRunStart command in the Backspace chain. When the caret is at the start of a run whose previous sibling is an inline structuredContent wrapper, Backspace now selects the wrapper as a NodeSelection (for unlocked / contentLocked modes) so a subsequent Backspace deletes it. For sdtLocked / sdtContentLocked wrappers the command consumes the keystroke without changing the selection. The structured-content select plugin ignores selections produced via the new meta flag so it does not collapse the NodeSelection back to a TextSelection.
Switch selectInlineSdtBeforeRunStart from a NodeSelection over the whole wrapper to a TextSelection over the inner content, and apply it uniformly across all lock modes. This avoids selecting the SDT chrome and keeps Backspace inside the field boundary.
When text inside an inline structured-content field changes, the containing paragraph's sdBlockRev was not incremented because nodesBetween over the replace range did not always visit the ancestor block. Walk up the ancestor chain at each changed range's boundaries so the block-level paragraph gets a fresh revision. Tag the plugin's own metadata transaction with a meta key so it neither re-triggers the block-node appendTransaction nor gets filtered out by the structured-content lock plugin.
When an exact content selection covers a contentLocked structured content field and the user presses Backspace/Delete, delete the wrapper directly instead of first promoting to a NodeSelection that required a second keystroke. Cut still promotes to NodeSelection so the browser can serialize the wrapper. Tests are updated to assert single-step deletion and now cover Delete in addition to Backspace.
Empty inline structured content used to be filtered out of the layout runs entirely, so the wrapper had no width, no caret target, and the field was effectively invisible. Introduce an `emptyInlineSdt` visual placeholder run that flows end-to-end through the pipeline: - contracts: `TextRun.visualPlaceholder` and an `isEmptyInlineSdtPlaceholderRun` guard; `sliceRunsForLine` preserves placeholders that have no chars. - pm-adapter: emit a placeholder run for inline `structuredContent` nodes with empty/missing content, and skip merging it into neighboring text. - measuring/dom: reserve an 8px inline box (0px for hidden-appearance) without taking the empty-paragraph code path. - painters/dom: render a `<span class="superdoc-empty-inline-sdt-placeholder">` inside the inline SDT wrapper, tagged `data-empty="true"`, and add styles so the wrapper gets a visible affordance without inflating line-box height. - DomSelectionGeometry: anchor the caret to the line's Y (the placeholder is height: 0) and always to its left edge. - structured-content lock plugin: Backspace/Delete inside an empty inline SDT deletes the wrapper when its lock mode allows it.
…int modes Remove CSS rules that blanked the ::before content for empty SDT placeholders in viewing and print modes so the placeholder prompt stays visible. Update tests to assert the rules are absent.
Import isValidImageDataUrl directly from @superdoc/url-validation instead of re-exporting through image-run, and use the StructuredContentMetadata type from contracts in place of its inlined shape.
Flip the drag-drop position assertions to expect the source to land before the drop anchor, and tighten the inline-SDT appearance spec to assert explicit boundingBox values are exposed while default appearance remains omitted.
luccas-harbour
force-pushed
the
luccas/sd-3237-bug-sdt-hover-and-click-to-place-cursor-interactions-are
branch
from
6b84f17 to
723f708
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 123 files
Partial review: This PR has more than 50 files, so cubic reviewed the highest-priority files first. During the trial, paid plans get a higher file limit.
You can try an ultrareview to bypass the file limit, comment @cubic-dev-ai ultrareview. Learn more.
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Fix all with cubic | Re-trigger cubic
Narrow line-content chrome bounds previously applied to multiline paragraphs and continuation fragments, shrinking the hover and click hit area below the visible block. Restrict that narrowing to single- line, complete fragments and expose a CSS variable so cross-fragment chrome can extend across the boundary.
Account for inline SDT wrapper chrome (4px each) when computing block SDT chrome bounds so nested inline SDTs don't get clipped by the outer block SDT frame.
caio-pizzol
deleted the
luccas/sd-3237-bug-sdt-hover-and-click-to-place-cursor-interactions-are
branch
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
4 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
Fixes SD-3237: SDT hover and click-to-place cursor interactions were unreliable. Clicking inside a structured content block highlighted the entire content on the first click (SD-1584's "first-click select-all"), repeated clicks didn't reliably reposition the caret, and the label/tab chrome appeared on hover before the user had actually focused the SDT. Nested SDTs amplified the jank.
This PR restores Word-like behavior: hover only greys the background, clicking places the caret at the clicked position (even inside nested SDTs), and label clicks select the whole control while still preserving the native drag-handle.
Changes
Pointer/selection behavior (
EditorInputManager.ts)select-all-on-first-clickbehavior on inline SDTs. Body clicks now place a collapsed caret like normal content.pointerdown/pointerup/clickon the owner document (visible host + viewport host + fallback throughelementsFromPoint), with a movement threshold so a label drag still triggers a nativedragstart.mouseup/click(notpointerdown), scoped to the editor that owns the hit label, and routed throughNodeSelectionwith apm-start-driven fallback. Gesture state is cleared onpointercancel.DOM contract (
dom-contract, painter, mapping)INLINE_SDT_LABEL,BLOCK_SDT_LABEL, plus aSTRUCTURED_CONTENT_CHROME_LABEL_CLASS_NAMESset). Painter,sdt-helpers, layout-bridgedom-mapping, andDomPointerMappingall use the contract instead of string literals.layout-bridge/dom-mapping.tsandDomPointerMapping.tsso theirpm-start/pm-endranges no longer bleed into body caret resolution.sdt-group-hoverstyle that revealed the block label on hover — hover only greys the background; the label appears on selection.Structured-content plugins
structured-content-select-plugin.js: removed the click-into-inline-SDT select-all behavior; now only handles boundary/exit navigation. Also tightened the "selects whole content" check so a parent block selection that contains a nested inline SDT no longer collapses to that inline.structured-content-lock-plugin.js: updated comments to reflect that the select-plugin no longer produces first-click select-all.Resolution
findStructuredContentBlockAtPosnow does adoc.nodeAt(pos)check first so block labels resolve at the node boundary (where the click lands on the chrome rather than inside the content).Tests
sd-3237-sdt-interactions.spec.tsbehavior spec +sd-3237-nested-sdt-lorem-ipsum.docxfixture covering:NodeSelection)sdt-drag-drop.spec.tsandstructured-content.spec.tsfor the new behavior.EditorInputManager.structuredContent.test.ts,DomPointerMapping.test.ts,structured-content-resolution.test.ts, and the lock/select plugin tests.