feat(document-api): selection primitives + multi-block comment targets (SD-2668)#2924
Merged
caio-pizzol merged 6 commits intoApr 23, 2026
Conversation
…s (SD-2668)
Adds `editor.doc.selection.current()` and `editor.doc.selection.onChange()` so
consumers can build custom toolbars, comments sidebars, and selection-driven
UIs without reaching into ProseMirror internals. Widens `comments.create`
target to accept `TextTarget` so multi-block selections anchor across blocks
instead of silently collapsing to the first segment.
- Consumers previously had to resolve PM positions through
`editor.state.doc`, walk the PM tree for the containing block's `sdBlockId`,
and convert to flattened-text offsets — ~30 lines of editor internals.
`selection.current()` returns a portable `SelectionInfo { empty, target,
activeMarks, text? }` with a multi-segment `TextTarget` ready for
`comments.create`.
- `comments.list().target` already used multi-segment `TextTarget`; the
write side (`comments.create`) only accepted single-block `TextAddress`,
so drag-selecting across paragraphs lost data with no warning.
- Contract, schemas, dispatch, and adapter wired. Super-editor adapter
projects PM selections into the flattened-text model via the existing
`computeTextContentLength` helper.
Part of the SD-2667 drop-in assessment umbrella.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a2c06c516
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1. Drop the aspirational `in: StoryLocator` parameter from
`selection.current`. The adapter always read the live editor selection
and merely copied `input.in` into the returned `TextTarget.story`, so a
body selection could be mislabeled as a header/footer selection. The
operation now documents that it always reflects whichever story holds
focus; story-scoped selection reads are out of scope.
2. Narrow `comments.create` multi-segment handling to contiguous ranges.
Validation previously accepted any non-empty segment array; the handler
spanned `first.start → last.end` as a single PM range, so disjoint or
out-of-order segments would silently anchor the comment over text the
caller never selected. `addCommentHandler` now resolves every segment,
rejects out-of-order pairs, and rejects any pair with non-empty text
between them (`INVALID_TARGET`).
3. Fix schema inconsistency: `SelectionInfo.target` is `TextTarget | null`
at the type level, but the published schema required it as an `object`.
Schema now uses `oneOf: [textTargetSchema, { type: 'null' }]` so empty
selections validate against the exported contract.
4. Make `SelectionAdapter` optional on `DocumentApiAdapters`. This is a
public exported interface; adding a required member is a source
breaking change for external adapter constructors. The factory now
throws `SELECTION_ADAPTER_UNAVAILABLE` when selection operations are
called without a registered adapter.
Tests: add 3 new cases (multi-segment forward, missing-adapter for
`current` + `onChange`). 1374 pass, 0 fail.
…flush Two bot findings on PR 2924: - P1: `collectTextSegments` was deriving block-relative offsets with `selStart - blockStart`, treating raw PM positions as flattened text offsets. That's only equivalent when the block contains pure text; it diverges whenever the block has inline wrappers (e.g. `run` marks) or leaf atoms whose PM boundary tokens do not count in the flattened model. A selection inside a run would therefore return a `TextTarget` off by the number of wrapper boundary tokens, and `comments.create` using that target would anchor to the wrong text. Added `pmPositionToTextOffset(blockNode, blockPos, pmPos)` alongside the existing `resolveTextRangeInBlock` in `text-offset-resolver.ts` — it walks the block with the same flattened model (text = length, leaf = 1, block separator = 1, inline wrapper tokens = 0) and returns the correct offset. `collectTextSegments` now uses it for both endpoints. - P2: `subscribeToSelection` scheduled `flush` via `queueMicrotask` but the returned unsubscribe only detached listeners — a microtask already queued before cleanup would still fire, invoking the listener after unsubscribe returned (stale state updates on component unmount). Added a `cancelled` closure flag set by unsubscribe and checked in `flush` and `schedule`. Tests: 5 new cases for `pmPositionToTextOffset` covering plain text, inline wrapper transparency, leaf atoms with `nodeSize > 1`, before- block-start, and past-block-end. 11515 super-editor pass, 0 fail.
Addresses PR review findings on test coverage: - Adds `selection-info-resolver.test.ts` (11 cases) covering `resolveCurrentSelectionInfo` projection (empty state, single-block selection, multi-block segment-per-touched-block, missing blockId, includeText on/off, active-marks empty path) and `subscribeToSelection` (listener fires once per tick, unsubscribe stops firing, queued-microtask-cancellation on unmount). - Adds 3 write-side cases to `comments-wrappers.test.ts` for the multi-segment path: out-of-order rejection with INVALID_TARGET / "document order" message, non-contiguous-gap rejection with "contiguous" message, and the contiguous-success path verifying the spanned PM range [first.from, last.to] is applied. - Updates `assemble-adapters.test.ts` to assert both `selection.current` and `selection.onChange` are wired on the adapter bag. - Adds a new section in `tests/consumer-typecheck/src/customer-scenario.ts` exercising the exported `editor.doc.selection.current()` surface, `SelectionInfo` destructuring, multi-segment `TextTarget` pass-through to `comments.create`, and the `onChange` subscription shape. Requires threading the new types through `packages/super-editor/src/index.ts` and `packages/superdoc/src/index.js` JSDoc typedefs so they are reachable from the `superdoc` package entrypoint. - Exempts `selection.onChange` from the contract-parity member-path check via `META_MEMBER_PATHS` — it is a subscription primitive, not a request/response operation, so it does not belong in `OPERATION_DEFINITIONS` / schemas / dispatch. Fixes the `validate` CI job that otherwise rejects the new runtime member. Deferred to SD-2671 (follow-up): - doc-api-stories/comments/multi-segment-target.ts (CLI-harness story) - doc-api-stories/selection story - Playwright behavior test that drives selection.current → comments.create Verified: document-api 1374 pass, super-editor 11529 pass, tests/consumer-typecheck compiles clean against the packed tarball.
…ment-api-selection-primitives-multi-block-comment # Conflicts: # apps/docs/document-api/reference/_generated-manifest.json
…ment-api-selection-primitives-multi-block-comment # Conflicts: # apps/docs/document-api/reference/_generated-manifest.json
caio-pizzol
deleted the
caio/sd-2668-featdocument-api-selection-primitives-multi-block-comment
branch
April 23, 2026 17:55
Contributor
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.47 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in vscode-ext v2.3.0-next.49 |
Contributor
|
🎉 This PR is included in esign v2.3.0-next.49 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc v2.0.0-next.1 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in template-builder v1.6.0-next.12 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-cli v1.0.0-next.1 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-sdk v2.0.0-next.1 |
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.
Adds
editor.doc.selection.current()andeditor.doc.selection.onChange()so consumers can build custom toolbars and comment sidebars without reaching into ProseMirror internals. Widenscomments.createtarget to accept a multi-segmentTextTarget, so a drag-selection across paragraphs now anchors correctly instead of silently collapsing to the first block.Why this first: the assessment app (SD-2667) showed consumers need ~30 lines of PM walking (
editor.state.doc.resolve(pos), block-id attr lookup, PM-position-to-flattened-offset math) to build what should be a one-liner. Andcomments.list().targetalready returned multi-segmentTextTarget, so the write-side single-block restriction was a read/write asymmetry, not an intentional scope.Trade-offs:
splitTargetSegmentsonly resolves first + last segments and spans a single PM range across them. Punt on discontinuous multi-range comments (not a real consumer ask yet; the engine model anchors a comment to one range).Verified:
pnpm --filter @superdoc/document-api test→ 1371 pass, 0 fail (includes 9 newisTextTargetcases + updated schema-error assertions)pnpm --filter @superdoc/super-editor test→ 11510 pass, 13 skippedpnpm run generate:all→ contract/docs/SDK regenerates clean; committed outputs reflect this PR onlyChild of SD-2667. Follow-ups in the umbrella: docs guide (SD-2669),
ranges.scrollIntoView(SD-2670), drop-in example + cleanups (SD-2671).