Skip to content

Commit d31eb43

Browse files
authored
feat(document-api): selection.current() exposes activeCommentIds and activeChangeIds (SD-2792) (#2981)
* feat(document-api): selection.current() exposes activeCommentIds and activeChangeIds (SD-2792) Adds two read-only id arrays to `SelectionInfo`: - `activeCommentIds: string[]` — `commentMark.commentId`s overlapping the selection (or under the caret when empty) - `activeChangeIds: string[]` — `trackInsert` / `trackDelete` / `trackFormat` mark ids overlapping the selection Union semantics (NOT intersection): an id is included when *any* character in the range carries the mark. `activeMarks` keeps its intersection semantics; the two answer different questions and have different defaults. Why: Custom sidebars need to answer "is there a comment / tracked change under the cursor?" to render a floating "comment here" hint, highlight the active sidebar card as the user moves the caret, disable a "new comment" button when the selection already covers an existing comment, and drive next/previous review navigation. Today consumers either: - Call `comments.list()` and overlap-filter on every keystroke (full read per cursor move), or - Reach into the rendered DOM via `document.querySelectorAll( '[data-comment-id]')` (the dropin-assessment lab does this — a DOM-shape coupling no migration guide should teach). Both are escape hatches. The selection resolver (SD-2668) already walks the selection range; collecting these ids is one extra pass on the same nodes. Implementation: - `collectActiveEntityIds` walks the selection in the same shape as `collectActiveMarks` (caret-only branch for empty selections plus stored marks; nodesBetween for ranges). Bounded allocation, runs in O(text nodes) for both branches. - Mark name constants are local to the resolver rather than imported from the comment / track-changes extensions. The resolver lives one package up the dependency graph; pulling in the PM plugins would bloat the import path for no gain. The constant set is small and changes when the schema does. - `selectionInfoKey` (transaction dedupe cache) now includes the new fields so a comment/change id change between transactions still emits to subscribers. Schema + generated reference docs updated; both fields are required and serialize as empty arrays when no entity marks are active. Verified: 23 resolver tests (5 new for entity-id collection covering union semantics, both kinds, mixed marks, JSON round-trip). Document- API: 1395 / 0 fail. Super-editor: 11893 / 13 skipped / 0 fail. Contract parity: 389/389. `pnpm run generate:all` clean. * fix(document-api): map activeChangeIds through canonical resolver Addresses PR #2981 review (P2). Tracked-change marks store `attrs.id` (the raw mark id), but `editor.doc.trackChanges.list()` returns canonical ids derived by `groupTrackedChanges` (a portable hash from change type + positions + author + date + excerpt). The first cut of `activeChangeIds` returned the raw ids directly. Consumers comparing them against `list().items[].id` to highlight the active review-sidebar card would silently get no match — every canonical id is a hex string, every raw id is whatever the document imported with. Fix: `mapRawChangeIdsToCanonical(editor, rawIds)` resolves through `groupTrackedChanges(editor)` (cached per `editor.state.doc`, so typical calls are O(grouped)) and translates raw → canonical. Unmapped raw ids (partial editor mid-construction, or marks that weren't grouped) are dropped rather than leaked, since exposing them would re-introduce the silent-no-match bug. `activeCommentIds` is unaffected — comment marks already carry their canonical `commentId` directly on `attrs`. Tests: - Existing change-ids test now uses `vi.mock` on `tracked-change-resolver.js` to seed the raw → canonical map and asserts the canonical ids come through. - New test for the defensive drop: an "orphan" raw id with no mapping must NOT appear in `activeChangeIds`. 24 resolver tests pass; super-editor 11894 / 13 skipped / 0 fail. * fix(document-api): importedId fallback + dedupe canonical change ids (PR #2981 review)
1 parent 7dee4a5 commit d31eb43

6 files changed

Lines changed: 426 additions & 7 deletions

File tree

apps/docs/document-api/reference/_generated-manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1031,5 +1031,5 @@
10311031
}
10321032
],
10331033
"marker": "{/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */}",
1034-
"sourceHash": "fa717df8cc013f9703b118596afe4cbbf655fe73f2e4e856588844110998ee2e"
1034+
"sourceHash": "20f0873e29e8589387d0776cc53e6eea8d5fce102a3eba9a47a183c49b5f0b13"
10351035
}

apps/docs/document-api/reference/selection/current.mdx

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ Returns a SelectionInfo with `empty`, `target` (TextTarget or null), `activeMark
4040

4141
| Field | Type | Required | Description |
4242
| --- | --- | --- | --- |
43+
| `activeChangeIds` | string[] | yes | |
44+
| `activeCommentIds` | string[] | yes | |
4345
| `activeMarks` | string[] | yes | |
4446
| `empty` | boolean | yes | |
4547
| `target` | TextTarget \\| null | yes | One of: TextTarget, null |
@@ -49,6 +51,12 @@ Returns a SelectionInfo with `empty`, `target` (TextTarget or null), `activeMark
4951

5052
```json
5153
{
54+
"activeChangeIds": [
55+
"example"
56+
],
57+
"activeCommentIds": [
58+
"example"
59+
],
5260
"activeMarks": [
5361
"example"
5462
],
@@ -99,6 +107,18 @@ Returns a SelectionInfo with `empty`, `target` (TextTarget or null), `activeMark
99107
{
100108
"additionalProperties": false,
101109
"properties": {
110+
"activeChangeIds": {
111+
"items": {
112+
"type": "string"
113+
},
114+
"type": "array"
115+
},
116+
"activeCommentIds": {
117+
"items": {
118+
"type": "string"
119+
},
120+
"type": "array"
121+
},
102122
"activeMarks": {
103123
"items": {
104124
"type": "string"
@@ -125,7 +145,9 @@ Returns a SelectionInfo with `empty`, `target` (TextTarget or null), `activeMark
125145
"required": [
126146
"empty",
127147
"target",
128-
"activeMarks"
148+
"activeMarks",
149+
"activeCommentIds",
150+
"activeChangeIds"
129151
],
130152
"type": "object"
131153
}

packages/document-api/src/contract/schemas.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5299,9 +5299,11 @@ const operationSchemas: Record<OperationId, OperationSchemaSet> = {
52995299
empty: { type: 'boolean' },
53005300
target: { oneOf: [textTargetSchema, { type: 'null' }] },
53015301
activeMarks: arraySchema({ type: 'string' }),
5302+
activeCommentIds: arraySchema({ type: 'string' }),
5303+
activeChangeIds: arraySchema({ type: 'string' }),
53025304
text: { type: 'string' },
53035305
},
5304-
['empty', 'target', 'activeMarks'],
5306+
['empty', 'target', 'activeMarks', 'activeCommentIds', 'activeChangeIds'],
53055307
),
53065308
},
53075309

packages/document-api/src/selection/selection.types.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,34 @@ export interface SelectionInfo {
4343
* Active marks at the caret or across the selection. Names are
4444
* ProseMirror mark type names (e.g. `'bold'`, `'italic'`, `'link'`).
4545
* Use these to drive toolbar active-state rendering.
46+
*
47+
* `activeMarks` uses **intersection** semantics — a name is present
48+
* only when every character in the selection carries that mark. This
49+
* matches Word/Google Docs toolbar behavior.
4650
*/
4751
activeMarks: string[];
52+
/**
53+
* Comment IDs whose `commentMark` overlaps any part of the current
54+
* selection (or covers the caret when empty). Use to drive a
55+
* floating "comment here" hint, highlight the active sidebar card,
56+
* or disable a "new comment" button when the selection already
57+
* covers an existing comment.
58+
*
59+
* **Union** semantics: an id is present when *any* character in the
60+
* selection carries that comment, not when every character does.
61+
* Multiple overlapping comments produce multiple ids.
62+
*/
63+
activeCommentIds: string[];
64+
/**
65+
* Tracked-change IDs whose `trackInsert` / `trackDelete` /
66+
* `trackFormat` mark overlaps any part of the current selection.
67+
* Same union semantics as {@link activeCommentIds}.
68+
*
69+
* Use to drive review-sidebar highlighting and next/previous
70+
* navigation without resolving every change individually via
71+
* `trackChanges.list()`.
72+
*/
73+
activeChangeIds: string[];
4874
/**
4975
* Quoted text of the selection. Populated only when `includeText: true`.
5076
* Undefined otherwise.

packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.test.ts

Lines changed: 234 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,23 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model';
33
import type { Editor } from '../../core/Editor.js';
44
import { resolveCurrentSelectionInfo, subscribeToSelection } from './selection-info-resolver.js';
55

6+
// Stub `groupTrackedChanges` so tests don't need a fully PM-shaped
7+
// editor with `editor.state.doc.textBetween` and the tracked-change
8+
// mark walker. Each test that exercises tracked-change ids configures
9+
// the raw → canonical mapping it expects.
10+
const groupTrackedChangesMock = vi.hoisted(() =>
11+
vi.fn(() => [] as Array<{ rawId: string; id: string; from: number; to: number }>),
12+
);
13+
vi.mock('./tracked-change-resolver.js', () => ({
14+
groupTrackedChanges: groupTrackedChangesMock,
15+
}));
16+
17+
const setTrackedChangeMapping = (mappings: Array<{ rawId: string; canonical: string }>) => {
18+
groupTrackedChangesMock.mockReturnValue(
19+
mappings.map((m) => ({ rawId: m.rawId, id: m.canonical, from: 0, to: 0 })) as never,
20+
);
21+
};
22+
623
// ---------------------------------------------------------------------------
724
// PM node stub builder
825
//
@@ -21,6 +38,12 @@ type NodeOptions = {
2138
attrs?: Record<string, unknown>;
2239
/** Mark names applied to this node (only used for text nodes). */
2340
markNames?: string[];
41+
/**
42+
* Marks with attrs (commentMark, trackInsert, etc). Coexists with
43+
* `markNames` — both end up in `node.marks`. Use this for tests that
44+
* exercise per-mark attribute-driven id collection.
45+
*/
46+
marksWithAttrs?: Array<{ name: string; attrs: Record<string, unknown> }>;
2447
};
2548

2649
function createNode(typeName: string, children: ProseMirrorNode[] = [], options: NodeOptions = {}): ProseMirrorNode {
@@ -50,7 +73,10 @@ function createNode(typeName: string, children: ProseMirrorNode[] = [], options:
5073
child(index: number) {
5174
return children[index]!;
5275
},
53-
marks: (options.markNames ?? []).map((name) => ({ type: { name } })),
76+
marks: [
77+
...(options.markNames ?? []).map((name) => ({ type: { name }, attrs: {} as Record<string, unknown> })),
78+
...(options.marksWithAttrs ?? []).map((m) => ({ type: { name: m.name }, attrs: m.attrs })),
79+
],
5480
// `nodesBetween` walks the whole subtree. A minimal correct
5581
// implementation for our test shapes: visit self first, then recurse
5682
// into children with the right child-position accounting.
@@ -154,7 +180,7 @@ describe('resolveCurrentSelectionInfo', () => {
154180
it('returns an empty info with null target when the editor has no state', () => {
155181
const editor = { state: null } as unknown as Editor;
156182
const info = resolveCurrentSelectionInfo(editor, {});
157-
expect(info).toEqual({ empty: true, target: null, activeMarks: [] });
183+
expect(info).toEqual({ empty: true, target: null, activeMarks: [], activeCommentIds: [], activeChangeIds: [] });
158184
});
159185

160186
it('projects a single-block selection into a one-segment TextTarget', () => {
@@ -325,6 +351,212 @@ describe('resolveCurrentSelectionInfo', () => {
325351
});
326352
});
327353

354+
// ---------------------------------------------------------------------------
355+
// activeCommentIds / activeChangeIds (SD-2792)
356+
// ---------------------------------------------------------------------------
357+
358+
/**
359+
* Marked-text helper that lets each run carry attribute-bearing marks
360+
* (commentMark with commentId, trackInsert/Delete/Format with id).
361+
*/
362+
function entityMarkedTextBlock(
363+
blockId: string,
364+
runs: Array<{
365+
text: string;
366+
marks?: string[];
367+
marksWithAttrs?: Array<{ name: string; attrs: Record<string, unknown> }>;
368+
}>,
369+
): ProseMirrorNode {
370+
const children = runs.map((r) =>
371+
createNode('text', [], {
372+
text: r.text,
373+
markNames: r.marks ?? [],
374+
marksWithAttrs: r.marksWithAttrs ?? [],
375+
}),
376+
);
377+
return createNode('paragraph', children, {
378+
isBlock: true,
379+
inlineContent: true,
380+
attrs: { sdBlockId: blockId },
381+
});
382+
}
383+
384+
describe('resolveCurrentSelectionInfo > entity ids', () => {
385+
it('collects commentIds from commentMarks across the selection (union)', () => {
386+
const docNode = doc([
387+
entityMarkedTextBlock('p1', [
388+
{ text: 'Hello ', marksWithAttrs: [{ name: 'commentMark', attrs: { commentId: 'c1' } }] },
389+
{
390+
text: 'world',
391+
marksWithAttrs: [
392+
{ name: 'commentMark', attrs: { commentId: 'c1' } },
393+
{ name: 'commentMark', attrs: { commentId: 'c2' } },
394+
],
395+
},
396+
]),
397+
]);
398+
// Select the whole text "Hello world" (PM positions 2..13).
399+
const editor = makeEditor(docNode, { from: 2, to: 13 });
400+
401+
const info = resolveCurrentSelectionInfo(editor, {});
402+
403+
expect([...info.activeCommentIds].sort()).toEqual(['c1', 'c2']);
404+
expect(info.activeChangeIds).toEqual([]);
405+
});
406+
407+
it('collects changeIds from trackInsert/trackDelete/trackFormat marks (translated through canonical resolver)', () => {
408+
// Raw mark ids and canonical Document API ids differ: the canonical
409+
// id is a derived hash from `groupTrackedChanges`. We mock that map
410+
// so the resolver sees raw 'tc1' / 'tc2' / 'tc3' and returns the
411+
// canonical 'tcA' / 'tcB' / 'tcC' that consumers see in
412+
// `trackChanges.list().items[].id`.
413+
setTrackedChangeMapping([
414+
{ rawId: 'tc1', canonical: 'tcA' },
415+
{ rawId: 'tc2', canonical: 'tcB' },
416+
{ rawId: 'tc3', canonical: 'tcC' },
417+
]);
418+
const docNode = doc([
419+
entityMarkedTextBlock('p1', [
420+
{ text: 'inserted ', marksWithAttrs: [{ name: 'trackInsert', attrs: { id: 'tc1' } }] },
421+
{ text: 'deleted ', marksWithAttrs: [{ name: 'trackDelete', attrs: { id: 'tc2' } }] },
422+
{ text: 'reformat', marksWithAttrs: [{ name: 'trackFormat', attrs: { id: 'tc3' } }] },
423+
]),
424+
]);
425+
const editor = makeEditor(docNode, { from: 2, to: 27 });
426+
427+
const info = resolveCurrentSelectionInfo(editor, {});
428+
429+
expect([...info.activeChangeIds].sort()).toEqual(['tcA', 'tcB', 'tcC']);
430+
expect(info.activeCommentIds).toEqual([]);
431+
});
432+
433+
it('drops raw change ids that have no canonical mapping (defensive)', () => {
434+
// Raw id present in the document but missing from groupTrackedChanges
435+
// (mid-construction editor, or a mark that wasn't grouped). Leaking
436+
// the raw id past the resolver would silently produce no-match
437+
// highlights in consumer sidebars; drop it instead.
438+
setTrackedChangeMapping([{ rawId: 'tc1', canonical: 'tcA' }]);
439+
const docNode = doc([
440+
entityMarkedTextBlock('p1', [
441+
{ text: 'mapped ', marksWithAttrs: [{ name: 'trackInsert', attrs: { id: 'tc1' } }] },
442+
{ text: 'orphan', marksWithAttrs: [{ name: 'trackInsert', attrs: { id: 'orphan-id' } }] },
443+
]),
444+
]);
445+
const editor = makeEditor(docNode, { from: 2, to: 14 });
446+
447+
const info = resolveCurrentSelectionInfo(editor, {});
448+
449+
expect(info.activeChangeIds).toEqual(['tcA']);
450+
});
451+
452+
it('dedupes canonical ids when two raw ids map to the same canonical (paired tracked changes)', () => {
453+
// Tracked replace produces paired insert + delete halves whose
454+
// raw mark ids both group to a single canonical id. A range
455+
// selection across both halves must surface the canonical id
456+
// once, not twice — otherwise sidebar counts and union-driven
457+
// highlights would double-count the change.
458+
setTrackedChangeMapping([
459+
{ rawId: 'tc1-insert', canonical: 'tcA' },
460+
{ rawId: 'tc1-delete', canonical: 'tcA' },
461+
]);
462+
const docNode = doc([
463+
entityMarkedTextBlock('p1', [
464+
{ text: 'inserted ', marksWithAttrs: [{ name: 'trackInsert', attrs: { id: 'tc1-insert' } }] },
465+
{ text: 'deleted', marksWithAttrs: [{ name: 'trackDelete', attrs: { id: 'tc1-delete' } }] },
466+
]),
467+
]);
468+
const editor = makeEditor(docNode, { from: 2, to: 16 });
469+
470+
const info = resolveCurrentSelectionInfo(editor, {});
471+
472+
expect(info.activeChangeIds).toEqual(['tcA']);
473+
});
474+
475+
it('reports both comment and change ids when a span carries both', () => {
476+
setTrackedChangeMapping([{ rawId: 'tc1', canonical: 'tcA' }]);
477+
const docNode = doc([
478+
entityMarkedTextBlock('p1', [
479+
{
480+
text: 'reviewed',
481+
marksWithAttrs: [
482+
{ name: 'commentMark', attrs: { commentId: 'c1' } },
483+
{ name: 'trackInsert', attrs: { id: 'tc1' } },
484+
],
485+
},
486+
]),
487+
]);
488+
const editor = makeEditor(docNode, { from: 2, to: 10 });
489+
490+
const info = resolveCurrentSelectionInfo(editor, {});
491+
492+
expect(info.activeCommentIds).toEqual(['c1']);
493+
expect(info.activeChangeIds).toEqual(['tcA']);
494+
});
495+
496+
it('returns empty id arrays when no entity marks overlap the selection', () => {
497+
const docNode = doc([entityMarkedTextBlock('p1', [{ text: 'Plain text', marks: ['bold'] }])]);
498+
const editor = makeEditor(docNode, { from: 2, to: 12 });
499+
500+
const info = resolveCurrentSelectionInfo(editor, {});
501+
502+
expect(info.activeCommentIds).toEqual([]);
503+
expect(info.activeChangeIds).toEqual([]);
504+
expect(info.activeMarks).toEqual(['bold']);
505+
});
506+
507+
it('uses union semantics, not intersection (one comment touching part of the selection counts)', () => {
508+
// Run 1 has comment c1; run 2 is plain. activeMarks would not include
509+
// a "bold" if it only touched run 1, but activeCommentIds should
510+
// include c1 because we use union semantics.
511+
const docNode = doc([
512+
entityMarkedTextBlock('p1', [
513+
{ text: 'commented', marksWithAttrs: [{ name: 'commentMark', attrs: { commentId: 'c1' } }] },
514+
{ text: ' tail', marks: [] },
515+
]),
516+
]);
517+
const editor = makeEditor(docNode, { from: 2, to: 16 });
518+
519+
const info = resolveCurrentSelectionInfo(editor, {});
520+
521+
expect(info.activeCommentIds).toEqual(['c1']);
522+
});
523+
524+
it('resolves comment ids from importedId / w:id when commentId is absent (legacy DOCX imports)', () => {
525+
// Imported / legacy comment marks may carry the id on
526+
// `importedId` or `w:id` instead of the post-import canonical
527+
// `commentId`. The resolver must honor the same fallback chain
528+
// the rest of the comment adapter graph uses
529+
// (`resolveCommentIdFromAttrs`); without it,
530+
// `selection.current().activeCommentIds` would stay empty over a
531+
// run that `comments.list()` reports as a real comment.
532+
const docNode = doc([
533+
entityMarkedTextBlock('p1', [
534+
{ text: 'imported ', marksWithAttrs: [{ name: 'commentMark', attrs: { importedId: 'imp-1' } }] },
535+
{ text: 'legacy', marksWithAttrs: [{ name: 'commentMark', attrs: { 'w:id': 'leg-2' } }] },
536+
]),
537+
]);
538+
const editor = makeEditor(docNode, { from: 2, to: 17 });
539+
540+
const info = resolveCurrentSelectionInfo(editor, {});
541+
542+
expect([...info.activeCommentIds].sort()).toEqual(['imp-1', 'leg-2']);
543+
});
544+
545+
it('empty arrays survive a JSON round-trip (serialization-stable shape)', () => {
546+
// Schema and dispatch tests assume the SelectionInfo output is JSON-
547+
// serializable with stable field presence. Empty arrays should
548+
// serialize and parse back as empty arrays, not be elided.
549+
const docNode = doc([textBlock('p1', 'Hello')]);
550+
const editor = makeEditor(docNode, { from: 2, to: 7 });
551+
552+
const info = resolveCurrentSelectionInfo(editor, {});
553+
const roundTripped = JSON.parse(JSON.stringify(info));
554+
555+
expect(roundTripped.activeCommentIds).toEqual([]);
556+
expect(roundTripped.activeChangeIds).toEqual([]);
557+
});
558+
});
559+
328560
// ---------------------------------------------------------------------------
329561
// subscribeToSelection
330562
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)