diff --git a/apps/docs/document-api/reference/_generated-manifest.json b/apps/docs/document-api/reference/_generated-manifest.json index d608f75fb8..6b2cfe73d2 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": "cdb0b02e84f6eb7f4db962c177d082e0f89ec48517abc775736a3d17e4da9ba8" + "sourceHash": "f8bc3155490940ebbfa68ecf81df0787bf5d7e4c0930bdaa599144974d093619" } 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 b8e05565af..665d0a5645 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -5295,9 +5295,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 b22d4bf9bb..e169d56c7e 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 @@ -21,6 +21,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 +56,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 +163,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', () => { @@ -324,3 +333,178 @@ describe('resolveCurrentSelectionInfo', () => { expect(elapsed).toBeLessThan(500); }); }); +// --------------------------------------------------------------------------- +// 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', () => { + 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(['tc1', 'tc2', 'tc3']); + expect(info.activeCommentIds).toEqual([]); + }); + + it('reports both comment and change ids when a span carries both', () => { + 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(['tc1']); + }); + + 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 + // adapter graph uses (`resolveCommentIdFromAttrs`). + 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('collects entity ids from inline atom nodes (image, tab, line break with marks)', () => { + // A range selection over an image with a comment should still + // surface the comment id. Inline leaf nodes carry marks just like + // text runs do; restricting to `isText` would skip them and leave + // sidebar highlighting empty for image-only / atom-only selections. + const imageWithComment = createNode('image', [], { + isInline: true, + isLeaf: true, + attrs: { src: 'cat.png' }, + marksWithAttrs: [{ name: 'commentMark', attrs: { commentId: 'c-img' } }], + }); + const block = createNode('paragraph', [imageWithComment], { + isBlock: true, + inlineContent: true, + attrs: { sdBlockId: 'p1' }, + }); + const docNode = doc([block]); + // Doc layout: doc[0..5], paragraph[1..4] (content [2..3]), image atom[2..3]. + // Select the image atom span: PM [2, 3]. + const editor = makeEditor(docNode, { from: 2, to: 3 }); + + const info = resolveCurrentSelectionInfo(editor, {}); + + expect(info.activeCommentIds).toEqual(['c-img']); + }); + + 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([]); + }); +}); 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 60201cc8e9..23fe3d16d9 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 @@ -2,6 +2,22 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model'; import type { SelectionCurrentInput, SelectionInfo, TextTarget, TextSegment } from '@superdoc/document-api'; import type { Editor } from '../../core/Editor.js'; import { pmPositionToTextOffset } from './text-offset-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 @@ -16,7 +32,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; @@ -29,11 +45,14 @@ 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: activeChangeIds } = collectActiveEntityIds(state, from, to); const info: SelectionInfo = { empty, target, activeMarks, + activeCommentIds, + activeChangeIds, }; if (input.includeText && !empty) { @@ -106,6 +125,72 @@ function readBlockId(node: ProseMirrorNode): string | null { return typeof id === 'string' && id.length > 0 ? id : null; } +/** + * 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). + */ +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 comments may carry the id on `importedId` or + // `w:id` instead of `commentId`. Use the same resolution helper + // the rest of the comment adapters use so a sidebar built on the + // selection slice highlights the active card for legacy DOCX + // imports too. + 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) => { + // Walk text nodes AND inline atoms (images, tabs, line breaks, + // footnote references). Inline leaf nodes can carry comment / + // tracked-change marks just like text runs do — limiting to + // `isText` would skip them and leave `activeCommentIds` / + // `activeChangeIds` empty for selections that only cover an + // image with a comment, breaking sidebar highlighting in that + // case. Block-level nodes still recurse so we descend into + // their inline children. + if (!node.isInline) 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, diff --git a/packages/super-editor/src/ui/comments.test.ts b/packages/super-editor/src/ui/comments.test.ts index becef18afb..2907e9b000 100644 --- a/packages/super-editor/src/ui/comments.test.ts +++ b/packages/super-editor/src/ui/comments.test.ts @@ -221,6 +221,35 @@ describe('ui.comments — snapshot', () => { ui.destroy(); }); + it('does not re-fire ui.comments.subscribe when the resolver returns fresh-but-equal activeCommentIds arrays', async () => { + // Post-SD-2792 the resolver returns `Array.from(new Set(...))` on + // every call — fresh references even when the contents are + // identical. Without slice-level memoization piping through the + // comments slice, every keystroke / selectionUpdate would trip + // shallowEqual and re-render every comment-aware sidebar. + const { superdoc, editor } = makeStubs({ comments: [{ id: 'c1', commentId: 'c1' }] }); + (editor.doc.selection.current as unknown as () => unknown) = vi.fn(() => ({ + empty: true, + target: null, + activeMarks: [], + activeCommentIds: ['c1'], // fresh array literal each call + activeChangeIds: [], + })); + const ui = createSuperDocUI({ superdoc }); + + const cb = vi.fn(); + ui.comments.subscribe(cb); + expect(cb).toHaveBeenCalledTimes(1); // initial + + superdoc.fireEditor('selectionUpdate'); + await Promise.resolve(); + superdoc.fireEditor('selectionUpdate'); + await Promise.resolve(); + + expect(cb).toHaveBeenCalledTimes(1); + ui.destroy(); + }); + it('refreshes the snapshot synchronously after own mutations (createFromSelection / resolve / delete)', () => { const target = { kind: 'text' as const, segments: [{ blockId: 'p1', range: { start: 0, end: 5 } }] }; const { superdoc, mocks } = makeStubs({ diff --git a/packages/super-editor/src/ui/create-super-doc-ui.test.ts b/packages/super-editor/src/ui/create-super-doc-ui.test.ts index 028be44f3e..add5bac05d 100644 --- a/packages/super-editor/src/ui/create-super-doc-ui.test.ts +++ b/packages/super-editor/src/ui/create-super-doc-ui.test.ts @@ -450,6 +450,119 @@ describe('createSuperDocUI', () => { expect(headerEditor.on).toHaveBeenCalled(); }); + it('state.selection mirrors full SelectionInfo (target, activeMarks, activeCommentIds, activeChangeIds, quotedText)', () => { + const superdoc = makeSuperdocStub(); + // Replace the default selection.current stub with one that returns + // the full SelectionInfo shape. + const target = { + kind: 'text' as const, + segments: [{ blockId: 'p1', range: { start: 0, end: 5 } }], + }; + (superdoc.activeEditor as { doc: { selection: { current: unknown } } }).doc.selection.current = vi.fn(() => ({ + empty: false, + text: 'Hello', + target, + activeMarks: ['bold', 'italic'], + activeCommentIds: ['c1'], + activeChangeIds: ['tc1'], + })); + const ui = createSuperDocUI({ superdoc }); + teardown.push(() => ui.destroy()); + + const slice = ui.select((state) => state.selection).get(); + expect(slice).toEqual({ + empty: false, + target, + activeMarks: ['bold', 'italic'], + activeCommentIds: ['c1'], + activeChangeIds: ['tc1'], + quotedText: 'Hello', + }); + }); + + it('state.selection slice keeps identity stable across recomputes when the projection has not changed', async () => { + const superdoc = makeSuperdocStub(); + const target = { + kind: 'text' as const, + segments: [{ blockId: 'p1', range: { start: 0, end: 5 } }], + }; + // Each call to selection.current returns FRESH arrays (mirrors the + // resolver behavior — `activeMarks`/`activeCommentIds`/`activeChangeIds` + // are produced per call, not memoized at the resolver level). + (superdoc.activeEditor as { doc: { selection: { current: unknown } } }).doc.selection.current = vi.fn(() => ({ + empty: false, + text: 'Hello', + target, + activeMarks: ['bold'], + activeCommentIds: ['c1'], + activeChangeIds: [], + })); + const ui = createSuperDocUI({ superdoc }); + teardown.push(() => ui.destroy()); + + const cb = vi.fn(); + ui.select((state) => state.selection, shallowEqual).subscribe(cb); + expect(cb).toHaveBeenCalledTimes(1); // initial + + // Fire two transactions that don't change the projection. Without + // slice-level memoization, shallowEqual on the slice would flip on + // every call because the inner arrays are fresh each time. + superdoc.fireEditor('transaction'); + await flushMicrotasks(); + superdoc.fireEditor('transaction'); + await flushMicrotasks(); + + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('state.selection slice changes identity when activeMarks change (typing into bold)', async () => { + const superdoc = makeSuperdocStub(); + let activeMarks: string[] = []; + (superdoc.activeEditor as { doc: { selection: { current: unknown } } }).doc.selection.current = vi.fn(() => ({ + empty: true, + text: '', + target: null, + activeMarks, + activeCommentIds: [], + activeChangeIds: [], + })); + const ui = createSuperDocUI({ superdoc }); + teardown.push(() => ui.destroy()); + + const cb = vi.fn(); + ui.select((state) => state.selection, shallowEqual).subscribe(cb); + expect(cb).toHaveBeenCalledTimes(1); + + activeMarks = ['bold']; + superdoc.fireEditor('selectionUpdate'); + await flushMicrotasks(); + + expect(cb).toHaveBeenCalledTimes(2); + const latestSlice = cb.mock.calls[1][0] as { activeMarks: string[] }; + expect(latestSlice.activeMarks).toEqual(['bold']); + }); + + it('state.selection falls back to safe defaults when selection.current is missing fields (legacy resolver)', () => { + const superdoc = makeSuperdocStub(); + // Legacy / partial resolver: only `empty` + `text` fields present. + (superdoc.activeEditor as { doc: { selection: { current: unknown } } }).doc.selection.current = vi.fn(() => ({ + empty: true, + text: '', + })); + const ui = createSuperDocUI({ superdoc }); + teardown.push(() => ui.destroy()); + + const slice = ui.select((state) => state.selection).get(); + expect(slice).toEqual({ + empty: true, + target: null, + activeMarks: [], + activeCommentIds: [], + activeChangeIds: [], + quotedText: '', + }); + }); + it('listener errors do not propagate to the editor or other subscribers', async () => { const superdoc = makeSuperdocStub(); const ui = createSuperDocUI({ superdoc }); diff --git a/packages/super-editor/src/ui/create-super-doc-ui.ts b/packages/super-editor/src/ui/create-super-doc-ui.ts index 22c4ad491c..959dc305c7 100644 --- a/packages/super-editor/src/ui/create-super-doc-ui.ts +++ b/packages/super-editor/src/ui/create-super-doc-ui.ts @@ -24,6 +24,7 @@ import type { ReviewHandle, ReviewItem, ReviewSlice, + SelectionSlice, SelectorFn, SuperDocEditorLike, SuperDocUI, @@ -55,7 +56,7 @@ const EDITOR_EVENTS = [ 'commentsUpdate', 'commentsLoaded', 'comment-positions', - 'trackedChangesUpdate', + 'tracked-changes-changed', ] as const; /** @@ -65,12 +66,14 @@ const EDITOR_EVENTS = [ * `scheduleNotify` for these, but we need the cache invalidation to * happen *first* so `computeState()` sees fresh items. * - * Includes `trackedChangesUpdate` so an external accept/reject (e.g., - * a collaborator's decision arriving over the wire, or a different - * UI mutating the list) refreshes the tracked-changes cache without - * waiting for an unrelated comments event. + * `tracked-changes-changed` is the canonical broadcast emitted by the + * tracked-change index whenever a transaction adds, removes, or + * invalidates tracked changes (including remote / collaborator-driven + * mutations). Without it, the cache only refreshes when the + * controller's own action methods call `refreshAndNotify`, leaving + * `ui.review` subscribers stale after normal editing. */ -const LIST_REFRESH_EVENTS = ['commentsUpdate', 'commentsLoaded', 'trackedChangesUpdate'] as const; +const LIST_REFRESH_EVENTS = ['commentsUpdate', 'commentsLoaded', 'tracked-changes-changed'] as const; const SUPERDOC_EVENTS = ['editorCreate', 'document-mode-change', 'zoomChange'] as const; @@ -140,6 +143,23 @@ function resolveRoutedEditor(superdoc: SuperDocUIOptions['superdoc']): SuperDocE } } +/** + * Resolve the **host** (body) editor — the one that owns the document + * scope. Always `superdoc.activeEditor`, never the routed + * header/footer/note story editor. + * + * Document-wide operations (`trackChanges.decide`, + * `presentation.navigateTo`, `presentation.scrollToPositionAsync`) + * must run against the host so the adapter treats the body as the + * scope and routes to the right story via the target's `story` + * field. Calling these on a child story editor (when focus is in a + * header/footer) would scope the decision/scroll to that story + * instead of the document. + */ +function resolveHostEditor(superdoc: SuperDocUIOptions['superdoc']): SuperDocEditorLike | null { + return (superdoc.activeEditor ?? null) as SuperDocEditorLike | null; +} + /** * Resolve the PresentationEditor (when one exists), so we can * subscribe to its events and re-route the active editor on surface @@ -306,6 +326,39 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { slice: ReviewSlice; } | null = null; + /** + * Memoized selection slice. Slice identity is stable when the + * derived shape — empty, target (deep), activeMarks, activeCommentIds, + * activeChangeIds, quotedText — has not changed since the last + * computeState. Without this, a typing-only transaction (which leaves + * the projected SelectionInfo unchanged but allocates fresh arrays + * inside the resolver) would re-fire every `ui.select(s => s.selection)` + * subscriber per keystroke. + */ + let selectionMemo: { key: string; slice: SelectionSlice } | null = null; + + /** + * Stable string key over a SelectionInfo for slice memoization. Two + * infos producing the same key represent the same observable + * selection state, so the slice can be reused. + */ + const buildSelectionKey = ( + empty: boolean, + target: import('@superdoc/document-api').TextTarget | null, + activeMarks: string[], + activeCommentIds: string[], + activeChangeIds: string[], + quotedText: string, + ): string => { + const targetKey = target + ? target.segments.map((s) => `${s.blockId}:${s.range.start}-${s.range.end}`).join('|') + : 'null'; + const marks = [...activeMarks].sort().join(','); + const comments = [...activeCommentIds].sort().join(','); + const changes = [...activeChangeIds].sort().join(','); + return `${empty ? '1' : '0'}:${targetKey}:m=${marks}:c=${comments}:tc=${changes}:t=${quotedText}`; + }; + const computeState = (): SuperDocUIState => { // Route through PresentationEditor when active so selection state // follows the body/header/footer/note editor the user is actually @@ -386,15 +439,54 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { }; } + // Build (or reuse) the rich selection slice. Memo key folds in + // every observable field so a typing-only transaction (which leaves + // the projected SelectionInfo unchanged but allocates fresh arrays + // inside the resolver) keeps the slice identity stable and lets + // `shallowEqual` short-circuit `ui.select(s => s.selection)` + // subscribers. + const selectionTarget = (selectionInfo?.target ?? null) as import('@superdoc/document-api').TextTarget | null; + const selectionActiveMarks = (selectionInfo?.activeMarks ?? EMPTY_ACTIVE_IDS) as string[]; + const selectionKey = buildSelectionKey( + empty, + selectionTarget, + selectionActiveMarks, + activeIds, + activeChangeIdsFromSelection, + quotedText, + ); + let selectionSlice: SelectionSlice; + if (selectionMemo && selectionMemo.key === selectionKey) { + selectionSlice = selectionMemo.slice; + } else { + selectionSlice = { + empty, + target: selectionTarget, + activeMarks: selectionActiveMarks, + activeCommentIds: activeIds, + activeChangeIds: activeChangeIdsFromSelection, + quotedText, + }; + selectionMemo = { key: selectionKey, slice: selectionSlice }; + } + return { ready, documentMode, - selection: { empty, quotedText }, + selection: selectionSlice, toolbar: toolbarSnapshot, comments: { total: commentsListCache.total, items: commentsListCache.items, - activeIds, + // Plumb from the memoized selection slice so the array + // reference stays stable across recomputes when the active + // set hasn't changed. The resolver returns a fresh `[]` (or + // a fresh non-empty array) every call; without this the + // `shallowEqual` check on `state.comments` would mismatch + // every transaction / selectionUpdate even when nothing in + // the comments slice actually changed, re-firing every + // `ui.comments.subscribe` listener on the editing hot path. + activeIds: selectionSlice.activeCommentIds, }, review: reviewSlice, }; @@ -673,13 +765,17 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { }; /** - * Run `scrollRangeIntoView` against whichever editor - * PresentationEditor currently routes to (body / header / footer / - * note). Returns `{ success: false }` when no routed editor is - * mounted. + * Run `scrollRangeIntoView` against the host editor — the + * presentation editor lives at the host level and its + * `navigateTo` is story-aware (the entity target's `story` field + * tells it which story to activate). Routing through a child story + * editor would scope navigation to that story instead of the + * document. + * + * Returns `{ success: false }` when no host editor is mounted. */ const runScrollIntoView = async (input: ScrollIntoViewInput): Promise => { - const editor = resolveRoutedEditor(superdoc); + const editor = resolveHostEditor(superdoc); if (!editor) return { success: false }; return scrollRangeIntoView(editor as unknown as Parameters[0], input); }; @@ -747,8 +843,16 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { return receipt; }, async scrollTo(commentId) { + // Preserve `address.story` for non-body comments so navigation + // resolves to the right header / footer / note rather than + // defaulting to body. + const story = lookupItemStory(commentId); + const target = + story != null + ? { kind: 'entity' as const, entityType: 'comment' as const, entityId: commentId, story } + : { kind: 'entity' as const, entityType: 'comment' as const, entityId: commentId }; return runScrollIntoView({ - target: { kind: 'entity', entityType: 'comment', entityId: commentId }, + target, block: 'center', behavior: 'smooth', }); @@ -764,7 +868,12 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { // from view mode. const requireDocTrackChanges = () => { - const editor = resolveRoutedEditor(superdoc); + // Always go through the host editor — `trackChanges.decide` is + // document-wide and the change's own `address.story` (carried in + // the decide target) tells the adapter which story to operate + // against. Routing through a child story editor when focus is in + // a header/footer would scope the decision to that story. + const editor = resolveHostEditor(superdoc); const api = editor?.doc?.trackChanges; if (!api?.decide) { throw new Error('ui.review: no active editor / trackChanges API. Open a document first.'); @@ -795,6 +904,23 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { return { id: changeId }; }; + /** + * Look up a review item's `address.story` so navigation / + * scrollTo can carry it into the EntityAddress target. Without this, + * `presentation.navigateTo({ entityId: 'tc-header-x' })` defaults + * to body and either fails with target-not-found or anchors to a + * same-id body change. Returns `undefined` for body-anchored items + * so the EntityAddress stays minimal. + */ + const lookupItemStory = (id: string): unknown | undefined => { + const change = trackChangesListCache.items.find((c) => c.id === id); + if (change) { + return (change as unknown as { address?: { story?: unknown } }).address?.story; + } + const comment = commentsListCache.items.find((c) => c.id === id); + return (comment as unknown as { address?: { story?: unknown } } | undefined)?.address?.story; + }; + const review: ReviewHandle = { getSnapshot: () => computeState().review, subscribe(listener) { @@ -867,8 +993,13 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { const entityType = kind === 'change' ? 'trackedChange' : 'comment'; activeReviewId = id; scheduleNotify(); + const story = lookupItemStory(id); + const target = + story != null + ? { kind: 'entity' as const, entityType, entityId: id, story } + : { kind: 'entity' as const, entityType, entityId: id }; return runScrollIntoView({ - target: { kind: 'entity', entityType, entityId: id }, + target, block: 'center', behavior: 'smooth', }); diff --git a/packages/super-editor/src/ui/review.test.ts b/packages/super-editor/src/ui/review.test.ts index 33243853fa..6f162ba4e8 100644 --- a/packages/super-editor/src/ui/review.test.ts +++ b/packages/super-editor/src/ui/review.test.ts @@ -415,8 +415,8 @@ describe('ui.review — regression: navigation persists past the selected item', }); }); -describe('ui.review — regression: trackedChangesUpdate refreshes cache', () => { - it('a trackedChangesUpdate event surfaces fresh items in the next snapshot', async () => { +describe('ui.review — regression: tracked-changes-changed refreshes cache', () => { + it('a tracked-changes-changed event surfaces fresh items in the next snapshot', async () => { const { superdoc } = makeStubs({ trackedChanges: [{ id: 'tc1' }], }); @@ -425,7 +425,11 @@ describe('ui.review — regression: trackedChangesUpdate refreshes cache', () => expect(ui.review.getSnapshot().items.map((i) => i.id)).toEqual(['tc1']); superdoc.setTrackedChanges([{ id: 'tc1' }, { id: 'tc2' }]); - superdoc.fireEditor('trackedChangesUpdate'); + // The tracked-change index broadcasts `tracked-changes-changed` + // (not `trackedChangesUpdate`) on every transaction that adds / + // removes / invalidates changes. The controller listens to that + // event so collaborator-driven mutations refresh the cache too. + superdoc.fireEditor('tracked-changes-changed'); await Promise.resolve(); expect(ui.review.getSnapshot().items.map((i) => i.id)).toEqual(['tc1', 'tc2']); @@ -484,6 +488,94 @@ describe('ui.review — regression: decide carries non-body story', () => { }); }); +describe('ui.review — regression: scrollTo carries non-body story', () => { + it('scrollTo on a header change passes target.story to navigateTo', async () => { + const { superdoc, mocks } = makeStubs({ + trackedChanges: [{ id: 'tc-header', story: 'header:rId1' }], + }); + const ui = createSuperDocUI({ superdoc }); + + await ui.review.scrollTo('tc-header'); + + expect(mocks.navigateTo).toHaveBeenCalledTimes(1); + expect(mocks.navigateTo).toHaveBeenCalledWith({ + kind: 'entity', + entityType: 'trackedChange', + entityId: 'tc-header', + story: 'header:rId1', + }); + ui.destroy(); + }); + + it('scrollTo on a body change omits target.story (parity with body-default)', async () => { + const { superdoc, mocks } = makeStubs({ + trackedChanges: [{ id: 'tc-body' }], + }); + const ui = createSuperDocUI({ superdoc }); + + await ui.review.scrollTo('tc-body'); + + expect(mocks.navigateTo).toHaveBeenCalledWith({ + kind: 'entity', + entityType: 'trackedChange', + entityId: 'tc-body', + }); + ui.destroy(); + }); +}); + +describe('ui.review — regression: decisions route through the host editor', () => { + it('accept(id) goes through superdoc.activeEditor (host) even when toolbar routing returns a child story editor', () => { + const { superdoc, mocks } = makeStubs({ + trackedChanges: [{ id: 'tc1' }], + }); + + // Plant a child story editor that the toolbar source resolver + // would return (simulating "focus is in a header"). Its decide + // mock must NEVER be called — review decisions are document-wide + // and must route through the host editor. + const childDecide = vi.fn((_input: unknown) => ({ success: false as const })); + const childEditor = { + doc: { trackChanges: { decide: childDecide } }, + }; + const hostEditor = superdoc.activeEditor as unknown as { + presentationEditor: { getActiveEditor: () => unknown }; + }; + hostEditor.presentationEditor.getActiveEditor = () => childEditor; + + const ui = createSuperDocUI({ superdoc }); + + ui.review.accept('tc1'); + + expect(mocks.decide).toHaveBeenCalledTimes(1); // host editor's decide + expect(childDecide).not.toHaveBeenCalled(); // child editor's decide untouched + + ui.destroy(); + }); + + it('acceptAll() routes through the host editor too', () => { + const { superdoc, mocks } = makeStubs({ + trackedChanges: [{ id: 'tc1' }, { id: 'tc2' }], + }); + const childDecide = vi.fn((_input: unknown) => ({ success: false as const })); + const hostEditor = superdoc.activeEditor as unknown as { + presentationEditor: { getActiveEditor: () => unknown }; + }; + hostEditor.presentationEditor.getActiveEditor = () => ({ + doc: { trackChanges: { decide: childDecide } }, + }); + + const ui = createSuperDocUI({ superdoc }); + + ui.review.acceptAll(); + + expect(mocks.decide).toHaveBeenCalledWith({ decision: 'accept', target: { scope: 'all' } }); + expect(childDecide).not.toHaveBeenCalled(); + + ui.destroy(); + }); +}); + describe('ui.review — regression: subscribers are not re-fired on unrelated transactions', () => { it('a typing-only event (transaction without comments/trackedChanges change) does not re-fire ui.review subscribers', async () => { const { superdoc } = makeStubs({ diff --git a/packages/super-editor/src/ui/scroll-into-view.ts b/packages/super-editor/src/ui/scroll-into-view.ts index 97650c194d..99b95321a4 100644 --- a/packages/super-editor/src/ui/scroll-into-view.ts +++ b/packages/super-editor/src/ui/scroll-into-view.ts @@ -66,10 +66,20 @@ export async function scrollRangeIntoView(editor: Editor, input: ScrollIntoViewI try { // After the entity early-return, `target` narrows to - // `TextAddress | TextTarget`. `TextTarget` has `segments`; - // `TextAddress` has `blockId` + `range`. Resolve the first - // segment and hand it to the text resolver. - const firstSegment = 'segments' in target ? target.segments[0] : { blockId: target.blockId, range: target.range }; + // `TextAddress | TextTarget`. Discriminate by checking for a + // non-empty `segments` array — a bare `'segments' in target` + // type-guard would mis-classify a hybrid payload that happens to + // carry both `segments` (empty) and `blockId`/`range` because + // `'segments' in {}` answers shape, not content. + const isMultiSegmentTarget = + Array.isArray((target as { segments?: unknown }).segments) && + ((target as { segments: unknown[] }).segments.length ?? 0) > 0; + const firstSegment = isMultiSegmentTarget + ? (target as { segments: Array<{ blockId: string; range: { start: number; end: number } }> }).segments[0] + : { + blockId: (target as { blockId: string }).blockId, + range: (target as { range: { start: number; end: number } }).range, + }; if (!firstSegment) return { success: false }; const resolved = resolveTextTarget(editor, { diff --git a/packages/super-editor/src/ui/types.ts b/packages/super-editor/src/ui/types.ts index 176df083ab..ff97a9393c 100644 --- a/packages/super-editor/src/ui/types.ts +++ b/packages/super-editor/src/ui/types.ts @@ -164,9 +164,54 @@ export interface SuperDocUIState { */ export type ToolbarSnapshotSlice = import('../headless-toolbar/types.js').ToolbarSnapshot; +/** + * Snapshot of the editor's current selection — the full + * {@link import('@superdoc/document-api').SelectionInfo} projection + * mirrored on the controller so a single `ui.select(s => s.selection, + * shallowEqual)` subscribe gives consumers everything they need to + * drive a floating bubble menu, format toolbar, mention popover, or + * "comment here" hint without dipping back into `editor.doc.selection.current()`. + */ export interface SelectionSlice { + /** True when the selection is empty (cursor only, no range). */ empty: boolean; - /** The selected text, or '' when the selection is collapsed. */ + /** + * The selection anchored to text content as a portable + * {@link import('@superdoc/document-api').TextTarget}, or `null` when + * the selection is not in text (empty document, node selection, no + * focus). Multi-segment when the selection spans multiple blocks. + * Pass directly to `editor.doc.comments.create({ target })`. + */ + target: import('@superdoc/document-api').TextTarget | null; + /** + * Active marks at the caret or across the selection. Names are + * ProseMirror mark type names (`'bold'`, `'italic'`, `'link'`). + * Drives toolbar active-state rendering. Intersection semantics: a + * mark name is included only if every character in the range carries + * it (or, when empty, the caret/stored marks). + */ + activeMarks: string[]; + /** + * Comment ids whose `commentMark` overlaps the selection (or sits + * under the caret when empty). Union semantics: an id is included + * when *any* character in the range carries the mark. Use to + * highlight the active sidebar card or render a "comment here" hint. + * Same array as `state.comments.activeIds` — duplicated for the + * single-subscribe ergonomic. + */ + activeCommentIds: string[]; + /** + * Tracked-change ids whose mark (`trackInsert` / `trackDelete` / + * `trackFormat`) overlaps the selection. Union semantics. Mirrors + * `state.review.activeId` (which picks the first id) for consumers + * that want the full set. + */ + activeChangeIds: string[]; + /** + * Quoted text of the selection. Always present on the slice; + * empty string when the selection is collapsed. Equivalent to + * `editor.doc.selection.current({ includeText: true }).text ?? ''`. + */ quotedText: string; }