diff --git a/apps/docs/document-api/reference/_generated-manifest.json b/apps/docs/document-api/reference/_generated-manifest.json index 3285758cff..a4708641f5 100644 --- a/apps/docs/document-api/reference/_generated-manifest.json +++ b/apps/docs/document-api/reference/_generated-manifest.json @@ -1031,5 +1031,5 @@ } ], "marker": "{/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */}", - "sourceHash": "fa717df8cc013f9703b118596afe4cbbf655fe73f2e4e856588844110998ee2e" + "sourceHash": "20f0873e29e8589387d0776cc53e6eea8d5fce102a3eba9a47a183c49b5f0b13" } diff --git a/apps/docs/document-api/reference/selection/current.mdx b/apps/docs/document-api/reference/selection/current.mdx index f9ff0d9d27..e64b6f7d44 100644 --- a/apps/docs/document-api/reference/selection/current.mdx +++ b/apps/docs/document-api/reference/selection/current.mdx @@ -40,6 +40,8 @@ Returns a SelectionInfo with `empty`, `target` (TextTarget or null), `activeMark | Field | Type | Required | Description | | --- | --- | --- | --- | +| `activeChangeIds` | string[] | yes | | +| `activeCommentIds` | string[] | yes | | | `activeMarks` | string[] | yes | | | `empty` | boolean | yes | | | `target` | TextTarget \\| null | yes | One of: TextTarget, null | @@ -49,6 +51,12 @@ Returns a SelectionInfo with `empty`, `target` (TextTarget or null), `activeMark ```json { + "activeChangeIds": [ + "example" + ], + "activeCommentIds": [ + "example" + ], "activeMarks": [ "example" ], @@ -99,6 +107,18 @@ Returns a SelectionInfo with `empty`, `target` (TextTarget or null), `activeMark { "additionalProperties": false, "properties": { + "activeChangeIds": { + "items": { + "type": "string" + }, + "type": "array" + }, + "activeCommentIds": { + "items": { + "type": "string" + }, + "type": "array" + }, "activeMarks": { "items": { "type": "string" @@ -125,7 +145,9 @@ Returns a SelectionInfo with `empty`, `target` (TextTarget or null), `activeMark "required": [ "empty", "target", - "activeMarks" + "activeMarks", + "activeCommentIds", + "activeChangeIds" ], "type": "object" } diff --git a/packages/document-api/src/contract/schemas.ts b/packages/document-api/src/contract/schemas.ts index d77ea0fdd1..31b8de69c9 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -5299,9 +5299,11 @@ const operationSchemas: Record = { empty: { type: 'boolean' }, target: { oneOf: [textTargetSchema, { type: 'null' }] }, activeMarks: arraySchema({ type: 'string' }), + activeCommentIds: arraySchema({ type: 'string' }), + activeChangeIds: arraySchema({ type: 'string' }), text: { type: 'string' }, }, - ['empty', 'target', 'activeMarks'], + ['empty', 'target', 'activeMarks', 'activeCommentIds', 'activeChangeIds'], ), }, diff --git a/packages/document-api/src/selection/selection.types.ts b/packages/document-api/src/selection/selection.types.ts index 0f04fd0f36..e14d86e8dd 100644 --- a/packages/document-api/src/selection/selection.types.ts +++ b/packages/document-api/src/selection/selection.types.ts @@ -43,8 +43,34 @@ export interface SelectionInfo { * Active marks at the caret or across the selection. Names are * ProseMirror mark type names (e.g. `'bold'`, `'italic'`, `'link'`). * Use these to drive toolbar active-state rendering. + * + * `activeMarks` uses **intersection** semantics — a name is present + * only when every character in the selection carries that mark. This + * matches Word/Google Docs toolbar behavior. */ activeMarks: string[]; + /** + * Comment IDs whose `commentMark` overlaps any part of the current + * selection (or covers the caret when empty). Use to drive a + * floating "comment here" hint, highlight the active sidebar card, + * or disable a "new comment" button when the selection already + * covers an existing comment. + * + * **Union** semantics: an id is present when *any* character in the + * selection carries that comment, not when every character does. + * Multiple overlapping comments produce multiple ids. + */ + activeCommentIds: string[]; + /** + * Tracked-change IDs whose `trackInsert` / `trackDelete` / + * `trackFormat` mark overlaps any part of the current selection. + * Same union semantics as {@link activeCommentIds}. + * + * Use to drive review-sidebar highlighting and next/previous + * navigation without resolving every change individually via + * `trackChanges.list()`. + */ + activeChangeIds: string[]; /** * Quoted text of the selection. Populated only when `includeText: true`. * Undefined otherwise. diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.test.ts index 2428c56479..3006fb886f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.test.ts @@ -3,6 +3,23 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model'; import type { Editor } from '../../core/Editor.js'; import { resolveCurrentSelectionInfo, subscribeToSelection } from './selection-info-resolver.js'; +// Stub `groupTrackedChanges` so tests don't need a fully PM-shaped +// editor with `editor.state.doc.textBetween` and the tracked-change +// mark walker. Each test that exercises tracked-change ids configures +// the raw → canonical mapping it expects. +const groupTrackedChangesMock = vi.hoisted(() => + vi.fn(() => [] as Array<{ rawId: string; id: string; from: number; to: number }>), +); +vi.mock('./tracked-change-resolver.js', () => ({ + groupTrackedChanges: groupTrackedChangesMock, +})); + +const setTrackedChangeMapping = (mappings: Array<{ rawId: string; canonical: string }>) => { + groupTrackedChangesMock.mockReturnValue( + mappings.map((m) => ({ rawId: m.rawId, id: m.canonical, from: 0, to: 0 })) as never, + ); +}; + // --------------------------------------------------------------------------- // PM node stub builder // @@ -21,6 +38,12 @@ type NodeOptions = { attrs?: Record; /** Mark names applied to this node (only used for text nodes). */ markNames?: string[]; + /** + * Marks with attrs (commentMark, trackInsert, etc). Coexists with + * `markNames` — both end up in `node.marks`. Use this for tests that + * exercise per-mark attribute-driven id collection. + */ + marksWithAttrs?: Array<{ name: string; attrs: Record }>; }; function createNode(typeName: string, children: ProseMirrorNode[] = [], options: NodeOptions = {}): ProseMirrorNode { @@ -50,7 +73,10 @@ function createNode(typeName: string, children: ProseMirrorNode[] = [], options: child(index: number) { return children[index]!; }, - marks: (options.markNames ?? []).map((name) => ({ type: { name } })), + marks: [ + ...(options.markNames ?? []).map((name) => ({ type: { name }, attrs: {} as Record })), + ...(options.marksWithAttrs ?? []).map((m) => ({ type: { name: m.name }, attrs: m.attrs })), + ], // `nodesBetween` walks the whole subtree. A minimal correct // implementation for our test shapes: visit self first, then recurse // into children with the right child-position accounting. @@ -154,7 +180,7 @@ describe('resolveCurrentSelectionInfo', () => { it('returns an empty info with null target when the editor has no state', () => { const editor = { state: null } as unknown as Editor; const info = resolveCurrentSelectionInfo(editor, {}); - expect(info).toEqual({ empty: true, target: null, activeMarks: [] }); + expect(info).toEqual({ empty: true, target: null, activeMarks: [], activeCommentIds: [], activeChangeIds: [] }); }); it('projects a single-block selection into a one-segment TextTarget', () => { @@ -325,6 +351,212 @@ describe('resolveCurrentSelectionInfo', () => { }); }); +// --------------------------------------------------------------------------- +// activeCommentIds / activeChangeIds (SD-2792) +// --------------------------------------------------------------------------- + +/** + * Marked-text helper that lets each run carry attribute-bearing marks + * (commentMark with commentId, trackInsert/Delete/Format with id). + */ +function entityMarkedTextBlock( + blockId: string, + runs: Array<{ + text: string; + marks?: string[]; + marksWithAttrs?: Array<{ name: string; attrs: Record }>; + }>, +): ProseMirrorNode { + const children = runs.map((r) => + createNode('text', [], { + text: r.text, + markNames: r.marks ?? [], + marksWithAttrs: r.marksWithAttrs ?? [], + }), + ); + return createNode('paragraph', children, { + isBlock: true, + inlineContent: true, + attrs: { sdBlockId: blockId }, + }); +} + +describe('resolveCurrentSelectionInfo > entity ids', () => { + it('collects commentIds from commentMarks across the selection (union)', () => { + const docNode = doc([ + entityMarkedTextBlock('p1', [ + { text: 'Hello ', marksWithAttrs: [{ name: 'commentMark', attrs: { commentId: 'c1' } }] }, + { + text: 'world', + marksWithAttrs: [ + { name: 'commentMark', attrs: { commentId: 'c1' } }, + { name: 'commentMark', attrs: { commentId: 'c2' } }, + ], + }, + ]), + ]); + // Select the whole text "Hello world" (PM positions 2..13). + const editor = makeEditor(docNode, { from: 2, to: 13 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + + expect([...info.activeCommentIds].sort()).toEqual(['c1', 'c2']); + expect(info.activeChangeIds).toEqual([]); + }); + + it('collects changeIds from trackInsert/trackDelete/trackFormat marks (translated through canonical resolver)', () => { + // Raw mark ids and canonical Document API ids differ: the canonical + // id is a derived hash from `groupTrackedChanges`. We mock that map + // so the resolver sees raw 'tc1' / 'tc2' / 'tc3' and returns the + // canonical 'tcA' / 'tcB' / 'tcC' that consumers see in + // `trackChanges.list().items[].id`. + setTrackedChangeMapping([ + { rawId: 'tc1', canonical: 'tcA' }, + { rawId: 'tc2', canonical: 'tcB' }, + { rawId: 'tc3', canonical: 'tcC' }, + ]); + const docNode = doc([ + entityMarkedTextBlock('p1', [ + { text: 'inserted ', marksWithAttrs: [{ name: 'trackInsert', attrs: { id: 'tc1' } }] }, + { text: 'deleted ', marksWithAttrs: [{ name: 'trackDelete', attrs: { id: 'tc2' } }] }, + { text: 'reformat', marksWithAttrs: [{ name: 'trackFormat', attrs: { id: 'tc3' } }] }, + ]), + ]); + const editor = makeEditor(docNode, { from: 2, to: 27 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + + expect([...info.activeChangeIds].sort()).toEqual(['tcA', 'tcB', 'tcC']); + expect(info.activeCommentIds).toEqual([]); + }); + + it('drops raw change ids that have no canonical mapping (defensive)', () => { + // Raw id present in the document but missing from groupTrackedChanges + // (mid-construction editor, or a mark that wasn't grouped). Leaking + // the raw id past the resolver would silently produce no-match + // highlights in consumer sidebars; drop it instead. + setTrackedChangeMapping([{ rawId: 'tc1', canonical: 'tcA' }]); + const docNode = doc([ + entityMarkedTextBlock('p1', [ + { text: 'mapped ', marksWithAttrs: [{ name: 'trackInsert', attrs: { id: 'tc1' } }] }, + { text: 'orphan', marksWithAttrs: [{ name: 'trackInsert', attrs: { id: 'orphan-id' } }] }, + ]), + ]); + const editor = makeEditor(docNode, { from: 2, to: 14 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + + expect(info.activeChangeIds).toEqual(['tcA']); + }); + + it('dedupes canonical ids when two raw ids map to the same canonical (paired tracked changes)', () => { + // Tracked replace produces paired insert + delete halves whose + // raw mark ids both group to a single canonical id. A range + // selection across both halves must surface the canonical id + // once, not twice — otherwise sidebar counts and union-driven + // highlights would double-count the change. + setTrackedChangeMapping([ + { rawId: 'tc1-insert', canonical: 'tcA' }, + { rawId: 'tc1-delete', canonical: 'tcA' }, + ]); + const docNode = doc([ + entityMarkedTextBlock('p1', [ + { text: 'inserted ', marksWithAttrs: [{ name: 'trackInsert', attrs: { id: 'tc1-insert' } }] }, + { text: 'deleted', marksWithAttrs: [{ name: 'trackDelete', attrs: { id: 'tc1-delete' } }] }, + ]), + ]); + const editor = makeEditor(docNode, { from: 2, to: 16 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + + expect(info.activeChangeIds).toEqual(['tcA']); + }); + + it('reports both comment and change ids when a span carries both', () => { + setTrackedChangeMapping([{ rawId: 'tc1', canonical: 'tcA' }]); + const docNode = doc([ + entityMarkedTextBlock('p1', [ + { + text: 'reviewed', + marksWithAttrs: [ + { name: 'commentMark', attrs: { commentId: 'c1' } }, + { name: 'trackInsert', attrs: { id: 'tc1' } }, + ], + }, + ]), + ]); + const editor = makeEditor(docNode, { from: 2, to: 10 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + + expect(info.activeCommentIds).toEqual(['c1']); + expect(info.activeChangeIds).toEqual(['tcA']); + }); + + it('returns empty id arrays when no entity marks overlap the selection', () => { + const docNode = doc([entityMarkedTextBlock('p1', [{ text: 'Plain text', marks: ['bold'] }])]); + const editor = makeEditor(docNode, { from: 2, to: 12 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + + expect(info.activeCommentIds).toEqual([]); + expect(info.activeChangeIds).toEqual([]); + expect(info.activeMarks).toEqual(['bold']); + }); + + it('uses union semantics, not intersection (one comment touching part of the selection counts)', () => { + // Run 1 has comment c1; run 2 is plain. activeMarks would not include + // a "bold" if it only touched run 1, but activeCommentIds should + // include c1 because we use union semantics. + const docNode = doc([ + entityMarkedTextBlock('p1', [ + { text: 'commented', marksWithAttrs: [{ name: 'commentMark', attrs: { commentId: 'c1' } }] }, + { text: ' tail', marks: [] }, + ]), + ]); + const editor = makeEditor(docNode, { from: 2, to: 16 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + + expect(info.activeCommentIds).toEqual(['c1']); + }); + + it('resolves comment ids from importedId / w:id when commentId is absent (legacy DOCX imports)', () => { + // Imported / legacy comment marks may carry the id on + // `importedId` or `w:id` instead of the post-import canonical + // `commentId`. The resolver must honor the same fallback chain + // the rest of the comment adapter graph uses + // (`resolveCommentIdFromAttrs`); without it, + // `selection.current().activeCommentIds` would stay empty over a + // run that `comments.list()` reports as a real comment. + const docNode = doc([ + entityMarkedTextBlock('p1', [ + { text: 'imported ', marksWithAttrs: [{ name: 'commentMark', attrs: { importedId: 'imp-1' } }] }, + { text: 'legacy', marksWithAttrs: [{ name: 'commentMark', attrs: { 'w:id': 'leg-2' } }] }, + ]), + ]); + const editor = makeEditor(docNode, { from: 2, to: 17 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + + expect([...info.activeCommentIds].sort()).toEqual(['imp-1', 'leg-2']); + }); + + it('empty arrays survive a JSON round-trip (serialization-stable shape)', () => { + // Schema and dispatch tests assume the SelectionInfo output is JSON- + // serializable with stable field presence. Empty arrays should + // serialize and parse back as empty arrays, not be elided. + const docNode = doc([textBlock('p1', 'Hello')]); + const editor = makeEditor(docNode, { from: 2, to: 7 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + const roundTripped = JSON.parse(JSON.stringify(info)); + + expect(roundTripped.activeCommentIds).toEqual([]); + expect(roundTripped.activeChangeIds).toEqual([]); + }); +}); + // --------------------------------------------------------------------------- // subscribeToSelection // --------------------------------------------------------------------------- diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.ts index 33cead0642..7789701561 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/selection-info-resolver.ts @@ -8,6 +8,23 @@ import type { } from '@superdoc/document-api'; import type { Editor } from '../../core/Editor.js'; import { pmPositionToTextOffset } from './text-offset-resolver.js'; +import { groupTrackedChanges } from './tracked-change-resolver.js'; +import { resolveCommentIdFromAttrs } from './value-utils.js'; + +/** + * Mark names that anchor live entities the UI cares about. We collect + * the entity ids in the same selection walk that produces + * `activeMarks` so consumers can answer "is there a comment / tracked + * change under the cursor?" without overlap-filtering `comments.list()` + * on every keystroke. + * + * Kept inline rather than imported from extension constants because + * the selection resolver lives one package up the dependency graph + * from the comment / track-changes extensions, and we'd rather not + * pull those (and their PM plugins) into the resolver's import graph. + */ +const COMMENT_MARK_NAME = 'commentMark'; +const TRACK_CHANGE_MARK_NAMES = new Set(['trackInsert', 'trackDelete', 'trackFormat']); /** * Reads the current ProseMirror selection and projects it into the Document @@ -22,7 +39,7 @@ import { pmPositionToTextOffset } from './text-offset-resolver.js'; export function resolveCurrentSelectionInfo(editor: Editor, input: SelectionCurrentInput): SelectionInfo { const state = editor.state; if (!state) { - return { empty: true, target: null, activeMarks: [] }; + return { empty: true, target: null, activeMarks: [], activeCommentIds: [], activeChangeIds: [] }; } const sel = state.selection; @@ -35,11 +52,23 @@ export function resolveCurrentSelectionInfo(editor: Editor, input: SelectionCurr const target: TextTarget | null = segments && segments.length > 0 ? buildTextTarget(segments) : null; const activeMarks = collectActiveMarks(state, from, to); + const { commentIds: activeCommentIds, changeIds: activeChangeRawIds } = collectActiveEntityIds(state, from, to); + + // Tracked-change marks store their PM `attrs.id` (raw id), but the + // Document API's canonical id (`trackChanges.list().items[].id`) is a + // derived hash from `groupTrackedChanges`. Consumers compare the + // active ids against `list()` output to highlight the active sidebar + // card; returning raw ids would silently miss every match. Translate + // raw → canonical here so `activeChangeIds` matches the public + // contract. + const activeChangeIds = mapRawChangeIdsToCanonical(editor, activeChangeRawIds); const info: SelectionInfo = { empty, target, activeMarks, + activeCommentIds, + activeChangeIds, }; if (input.includeText && !empty) { @@ -112,6 +141,112 @@ function readBlockId(node: ProseMirrorNode): string | null { return typeof id === 'string' && id.length > 0 ? id : null; } +/** + * Translate raw PM-mark `attrs.id`s to the canonical Document API + * tracked-change ids that `trackChanges.list()` returns. + * + * `groupTrackedChanges(editor)` is the single source of truth for the + * raw → canonical mapping; it's already cached per + * `editor.state.doc`, so a typical selection.current() call hits the + * cache and runs O(grouped). Unmapped raw ids (a partial editor or + * a mark that wasn't grouped for some reason) are dropped from the + * result rather than emitted as raw — leaking raw ids past this point + * would re-introduce the silent-no-match bug consumers report. + */ +function mapRawChangeIdsToCanonical(editor: Editor, rawIds: string[]): string[] { + if (rawIds.length === 0) return rawIds; + let grouped: ReturnType; + try { + grouped = groupTrackedChanges(editor); + } catch { + // Defensive: a partial editor mid-tear-down shouldn't wedge + // selection.current(). Fall back to dropping the change ids. + return []; + } + const rawToCanonical = new Map(); + for (const change of grouped) { + rawToCanonical.set(change.rawId, change.id); + } + // Dedupe through a Set: when two raw ids in `rawIds` group to the + // same canonical (e.g. paired tracked-change pieces — insert + delete + // halves of a tracked replace, or an undo step that produced a stale + // raw alias), the canonical should appear once. Without this, an + // overlapping selection across both halves would emit a duplicate + // in `activeChangeIds` and double-count in any UI driven by it. + const seen = new Set(); + const out: string[] = []; + for (const raw of rawIds) { + const canonical = rawToCanonical.get(raw); + if (!canonical) continue; + if (seen.has(canonical)) continue; + seen.add(canonical); + out.push(canonical); + } + return out; +} + +/** + * Collect comment and tracked-change ids that touch the selection. + * + * Union semantics (NOT intersection): an id is included when *any* + * character in the range carries that mark. For an empty selection + * (caret), this resolves to ids on the marks at the caret position, + * including stored marks the user is about to apply. + * + * Walks text nodes in one pass; bounded allocation. Co-located with + * `collectActiveMarks` so the resolver only walks the selection + * range twice (once for mark-name intersection, once here for + * id-attribute union) — the dedup at the subscriber level + * (`selectionInfoKey`) keeps this cheap on the hot path. + */ +function collectActiveEntityIds( + state: { selection: any; storedMarks?: any; doc: ProseMirrorNode }, + from: number, + to: number, +): { commentIds: string[]; changeIds: string[] } { + const commentIds = new Set(); + const changeIds = new Set(); + + const collectFromMark = (markType: string, attrs: Record | undefined) => { + if (markType === COMMENT_MARK_NAME) { + // Imported / legacy comment marks may carry the id on + // `importedId` or `w:id` instead of `commentId`. The rest of + // the comment adapter graph (`comments.list`, `comments.patch`, + // etc.) treats those as the canonical id; without the same + // fallback, `selection.current().activeCommentIds` would stay + // empty over an imported anchor while `comments.list` reports + // the comment — breaking sidebar highlight / disable logic for + // legacy DOCX imports. + const id = resolveCommentIdFromAttrs((attrs ?? {}) as Record); + if (typeof id === 'string' && id.length > 0) commentIds.add(id); + } else if (TRACK_CHANGE_MARK_NAMES.has(markType)) { + const id = attrs?.id; + if (typeof id === 'string' && id.length > 0) changeIds.add(id); + } + }; + + if (from === to) { + // Caret-only: include stored marks (sticky formatting the user is + // about to apply) plus the marks resolved at the position itself. + if (state.storedMarks) { + for (const mark of state.storedMarks) collectFromMark(mark.type.name, mark.attrs); + } + const $pos = state.doc.resolve(from); + for (const mark of $pos.marks()) collectFromMark(mark.type.name, mark.attrs); + } else { + state.doc.nodesBetween(from, to, (node, pos) => { + if (!node.isText) return true; + const start = Math.max(pos, from); + const end = Math.min(pos + node.nodeSize, to); + if (end <= start) return false; + for (const mark of node.marks) collectFromMark(mark.type.name, mark.attrs); + return false; + }); + } + + return { commentIds: Array.from(commentIds), changeIds: Array.from(changeIds) }; +} + function collectActiveMarks( state: { selection: any; storedMarks?: any; doc: ProseMirrorNode }, from: number, @@ -195,7 +330,9 @@ function selectionInfoKey(info: SelectionInfo): string { targetKey = target.segments.map((s) => `${s.blockId}:${s.range.start}-${s.range.end}`).join('|'); } const marks = [...info.activeMarks].sort().join(','); - return `${info.empty ? '1' : '0'}:${targetKey}:${marks}`; + const comments = [...info.activeCommentIds].sort().join(','); + const changes = [...info.activeChangeIds].sort().join(','); + return `${info.empty ? '1' : '0'}:${targetKey}:${marks}:c=${comments}:tc=${changes}`; } function markTypesPresentEverywhere(doc: ProseMirrorNode, from: number, to: number): Set {