diff --git a/packages/super-editor/src/index.ts b/packages/super-editor/src/index.ts index 24b21d01a2..19b282b7f3 100644 --- a/packages/super-editor/src/index.ts +++ b/packages/super-editor/src/index.ts @@ -168,6 +168,9 @@ export type { CommentsHandle, CommentsSlice, EqualityFn, + ReviewHandle, + ReviewItem, + ReviewSlice, SelectorFn, SelectionSlice, Subscribable, 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 bd689480b9..5bd75a8b1a 100644 --- a/packages/super-editor/src/ui/create-super-doc-ui.ts +++ b/packages/super-editor/src/ui/create-super-doc-ui.ts @@ -7,14 +7,16 @@ import type { PublicToolbarItemId, ToolbarSnapshot, } from '../headless-toolbar/types.js'; -import type { CommentsListResult, Receipt, ScrollIntoViewOutput } from '@superdoc/document-api'; +import type { CommentsListResult, Receipt, ScrollIntoViewOutput, TrackChangesListResult } from '@superdoc/document-api'; import { shallowEqual } from './equality.js'; import type { CommandHandle, CommandsHandle, CommentsHandle, - CommentsSlice, EqualityFn, + ReviewHandle, + ReviewItem, + ReviewSlice, SelectorFn, SuperDocEditorLike, SuperDocUI, @@ -47,12 +49,17 @@ const EDITOR_EVENTS = [ /** * Editor events that should trigger a refresh of the cached - * `comments.list()` result before notifying subscribers. The base - * `EDITOR_EVENTS` list also fires `scheduleNotify` for these, but we - * need the cache invalidation to happen *first* so `computeState()` - * sees fresh items. + * `comments.list()` / `trackChanges.list()` results before notifying + * subscribers. The base `EDITOR_EVENTS` list also fires + * `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. */ -const COMMENTS_REFRESH_EVENTS = ['commentsUpdate', 'commentsLoaded'] as const; +const LIST_REFRESH_EVENTS = ['commentsUpdate', 'commentsLoaded', 'trackedChangesUpdate'] as const; const SUPERDOC_EVENTS = ['editorCreate', 'document-mode-change', 'zoomChange'] as const; @@ -90,9 +97,7 @@ const FALLBACK_COMMAND_STATE: ToolbarCommandHandleState = { * `createToolbarRegistry()`. Future custom-command registration * (FRICTION S3) will need to extend this dynamically. */ -const ALL_TOOLBAR_COMMAND_IDS: PublicToolbarItemId[] = Object.keys( - createToolbarRegistry(), -) as PublicToolbarItemId[]; +const ALL_TOOLBAR_COMMAND_IDS: PublicToolbarItemId[] = Object.keys(createToolbarRegistry()) as PublicToolbarItemId[]; /** * Frozen empty-array sentinel for `state.comments.activeIds` when @@ -229,6 +234,67 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { }; refreshCommentsListCache(); + // Tracked-changes cache. Same posture as comments — refresh on + // commentsUpdate / trackedChangesUpdate (track-changes events ride + // commentsUpdate today; the controller normalizes that for callers). + // `in: 'all'` is requested so non-body stories (header, footer, + // footnote, endnote) are included in the merged review feed. + const EMPTY_TRACK_CHANGES_LIST: TrackChangesListResult = { + evaluatedRevision: '', + total: 0, + items: [], + page: { limit: 0, offset: 0, returned: 0 }, + }; + let trackChangesListCache: TrackChangesListResult = EMPTY_TRACK_CHANGES_LIST; + const refreshTrackChangesListCache = () => { + const editor = resolveRoutedEditor(superdoc); + const list = editor?.doc?.trackChanges?.list; + if (typeof list !== 'function') { + trackChangesListCache = EMPTY_TRACK_CHANGES_LIST; + return; + } + try { + const result = list.call(editor.doc!.trackChanges, { in: 'all' }) as TrackChangesListResult | undefined; + trackChangesListCache = result ?? EMPTY_TRACK_CHANGES_LIST; + } catch { + // See refreshCommentsListCache rationale: cross-document leakage + // would be worse than briefly empty. + trackChangesListCache = EMPTY_TRACK_CHANGES_LIST; + } + }; + refreshTrackChangesListCache(); + + /** + * Internal `activeReviewId`. Mirrors selection-driven activity when + * the user moves the cursor to a different review item, and is + * updated by explicit `ui.review.next/previous/scrollTo` calls. + * Tracked separately from `lastSelectionDrivenId` so explicit + * navigation away from a still-selected item isn't immediately + * overwritten by the next computeState() call. + */ + let activeReviewId: string | null = null; + /** + * The selection-driven id observed during the last `computeState`. + * Only when this changes between calls does the controller mirror + * it onto `activeReviewId`; otherwise the user's `next() / + * previous() / scrollTo()` choice persists across recomputes. + */ + let lastSelectionDrivenId: string | null = null; + + /** + * Memoized review slice. The merged-feed array is rebuilt only when + * one of its inputs changes — comments items reference, tracked- + * changes items reference, or `activeReviewId`. Without this, + * shallowEqual on `state.review` would mismatch every keystroke + * because we'd allocate a fresh items array per computeState. + */ + let reviewMemo: { + commentsRef: CommentsListResult['items'] | null; + changesRef: TrackChangesListResult['items'] | null; + activeId: string | null; + slice: ReviewSlice; + } | null = null; + const computeState = (): SuperDocUIState => { // Route through PresentationEditor when active so selection state // follows the body/header/footer/note editor the user is actually @@ -246,6 +312,69 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { // computeState() calls (otherwise shallowEqual on the comments // snapshot re-fires every selection event). const activeIds = (selectionInfo?.activeCommentIds ?? EMPTY_ACTIVE_IDS) as string[]; + const activeChangeIdsFromSelection = (selectionInfo?.activeChangeIds ?? EMPTY_ACTIVE_IDS) as string[]; + + // Reconcile activeReviewId. Mirror selection only when the + // *selection-driven* id has changed since the last computeState — + // otherwise an explicit next/previous/scrollTo is preserved across + // subsequent recomputes (the cursor hasn't moved). Sync logic: + // - selection moved to a non-null entity id → mirror it + // - selection moved to no entity (caret elsewhere) → keep + // activeReviewId so navigation persists, but clear it if the + // underlying item dropped out of the feed + const selectionDrivenActiveId = activeIds[0] ?? activeChangeIdsFromSelection[0] ?? null; + const selectionMoved = selectionDrivenActiveId !== lastSelectionDrivenId; + lastSelectionDrivenId = selectionDrivenActiveId; + if (selectionMoved && selectionDrivenActiveId) { + activeReviewId = selectionDrivenActiveId; + } + + // Build (or reuse) the merged review feed. Memo invalidates only + // when source caches or activeReviewId change, so unrelated + // transactions / selection events don't allocate a fresh items + // array and re-fire ui.review subscribers. + let reviewSlice: ReviewSlice; + if ( + reviewMemo && + reviewMemo.commentsRef === commentsListCache.items && + reviewMemo.changesRef === trackChangesListCache.items && + reviewMemo.activeId === activeReviewId + ) { + reviewSlice = reviewMemo.slice; + } else { + const items: ReviewItem[] = []; + let order = 0; + for (const comment of commentsListCache.items) { + // `comments.list()` returns `DiscoveryItem` whose + // canonical identifier lives on `id` (set from the underlying + // commentId by the adapter). The legacy `commentId` field is + // only on `CommentInfo` / `comments.get()` — not on this + // discovery shape. Reading it would emit `undefined` and break + // active-id matching + next/previous/scrollTo. + items.push({ kind: 'comment', id: comment.id, documentOrder: order++, comment }); + } + for (const change of trackChangesListCache.items) { + items.push({ kind: 'change', id: change.id, documentOrder: order++, change }); + } + let openCount = trackChangesListCache.total; + for (const c of commentsListCache.items) { + if (c.status !== 'resolved') openCount += 1; + } + // If the previously active id dropped out of the feed (e.g. an + // accept/delete/reject), reset to null. Compute *after* items is + // built so the final slice matches the eventual activeReviewId. + if (activeReviewId && !items.some((item) => item.id === activeReviewId)) { + activeReviewId = null; + } + reviewSlice = { items, openCount, activeId: activeReviewId }; + reviewMemo = { + commentsRef: commentsListCache.items, + changesRef: trackChangesListCache.items, + activeId: activeReviewId, + slice: reviewSlice, + }; + } + return { ready, documentMode, @@ -256,6 +385,7 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { items: commentsListCache.items, activeIds, }, + review: reviewSlice, }; }; @@ -281,6 +411,7 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { const refreshAndNotify = () => { refreshCommentsListCache(); + refreshTrackChangesListCache(); scheduleNotify(); }; @@ -299,12 +430,12 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { // subsequent state recompute sees the fresh items array. Without // this, `state.comments.items` would lag one tick behind a create/ // patch/delete. - COMMENTS_REFRESH_EVENTS.forEach((name) => { + LIST_REFRESH_EVENTS.forEach((name) => { next.on?.(name, refreshAndNotify); }); currentEditorTeardown = () => { EDITOR_EVENTS.forEach((name) => next.off?.(name, scheduleNotify)); - COMMENTS_REFRESH_EVENTS.forEach((name) => next.off?.(name, refreshAndNotify)); + LIST_REFRESH_EVENTS.forEach((name) => next.off?.(name, refreshAndNotify)); }; // The set of source events changed and the routed editor swapped // — refresh the comments cache for the new editor and recompute @@ -611,6 +742,140 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { }, }; + // ---- ui.review ---------------------------------------------------------- + // + // Same architectural rules as `ui.comments`: every mutation routes + // through the Document API (`editor.doc.trackChanges.decide`); next + // / previous / scrollTo are UI-only navigation helpers; setRecording + // is a temporary documentMode flip until SD-2667/S4 splits recording + // from view mode. + + const requireDocTrackChanges = () => { + const editor = resolveRoutedEditor(superdoc); + const api = editor?.doc?.trackChanges; + if (!api?.decide) { + throw new Error('ui.review: no active editor / trackChanges API. Open a document first.'); + } + return api; + }; + + /** Determine the entity kind for a given id from the current feed. */ + const entityKindForId = (id: string): 'comment' | 'change' | null => { + const feed = computeState().review.items; + const item = feed.find((i) => i.id === id); + return item?.kind ?? null; + }; + + /** + * Build the `target` payload for `trackChanges.decide` for a single + * change id. Looks up the change in the cached feed; when its + * `address.story` is non-body (header / footer / footnote / + * endnote), include the story so the doc-API adapter can route + * the decision to the right story instead of defaulting to body and + * failing with target-not-found. Body-anchored changes omit the + * field for parity with the doc-API's body-default contract. + */ + const buildChangeDecideTarget = (changeId: string): { id: string; story?: unknown } => { + const item = trackChangesListCache.items.find((c) => c.id === changeId); + const story = (item as unknown as { address?: { story?: unknown } } | undefined)?.address?.story; + if (story != null) return { id: changeId, story }; + return { id: changeId }; + }; + + const review: ReviewHandle = { + getSnapshot: () => computeState().review, + subscribe(listener) { + return select((state) => state.review, shallowEqual).subscribe((snapshot) => { + try { + listener({ snapshot }); + } catch { + // see scheduleNotify + } + }); + }, + accept(changeId) { + const api = requireDocTrackChanges(); + const receipt = (api.decide as (input: unknown, options?: unknown) => Receipt).call(api, { + decision: 'accept', + target: buildChangeDecideTarget(changeId), + }); + refreshAndNotify(); + return receipt; + }, + reject(changeId) { + const api = requireDocTrackChanges(); + const receipt = (api.decide as (input: unknown, options?: unknown) => Receipt).call(api, { + decision: 'reject', + target: buildChangeDecideTarget(changeId), + }); + refreshAndNotify(); + return receipt; + }, + acceptAll() { + const api = requireDocTrackChanges(); + const receipt = (api.decide as (input: unknown, options?: unknown) => Receipt).call(api, { + decision: 'accept', + target: { scope: 'all' }, + }); + refreshAndNotify(); + return receipt; + }, + rejectAll() { + const api = requireDocTrackChanges(); + const receipt = (api.decide as (input: unknown, options?: unknown) => Receipt).call(api, { + decision: 'reject', + target: { scope: 'all' }, + }); + refreshAndNotify(); + return receipt; + }, + next() { + const items = computeState().review.items; + if (items.length === 0) return null; + const current = activeReviewId ? items.findIndex((i) => i.id === activeReviewId) : -1; + // Wrap-around: after last → first; null active → first. + const nextIndex = current < 0 || current >= items.length - 1 ? 0 : current + 1; + activeReviewId = items[nextIndex]!.id; + scheduleNotify(); + return activeReviewId; + }, + previous() { + const items = computeState().review.items; + if (items.length === 0) return null; + const current = activeReviewId ? items.findIndex((i) => i.id === activeReviewId) : -1; + // Wrap-around: before first → last; null active → last. + const prevIndex = current <= 0 ? items.length - 1 : current - 1; + activeReviewId = items[prevIndex]!.id; + scheduleNotify(); + return activeReviewId; + }, + async scrollTo(id) { + const kind = entityKindForId(id); + const entityType = kind === 'change' ? 'trackedChange' : 'comment'; + const api = requireDocRanges(); + activeReviewId = id; + scheduleNotify(); + return (api.scrollIntoView as (input: unknown) => Promise).call(api, { + target: { kind: 'entity', entityType, entityId: id }, + block: 'center', + behavior: 'smooth', + }); + }, + setRecording(enabled) { + // SD-2667/S4 will introduce an independent + // `editor.doc.trackChanges.setRecording` primitive. Until then, + // recording state is collapsed onto `documentMode` — flip + // between 'suggesting' (recording on) and 'editing' (off). + const next = enabled ? 'suggesting' : 'editing'; + if (typeof superdoc.setDocumentMode === 'function') { + superdoc.setDocumentMode(next); + } else if (superdoc.config) { + superdoc.config.documentMode = next; + } + scheduleNotify(); + }, + }; + const destroy = () => { if (destroyed) return; destroyed = true; @@ -627,5 +892,5 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { teardown.length = 0; }; - return { select, toolbar, commands, comments, destroy }; + return { select, toolbar, commands, comments, review, destroy }; } diff --git a/packages/super-editor/src/ui/index.ts b/packages/super-editor/src/ui/index.ts index efd055f401..987d366015 100644 --- a/packages/super-editor/src/ui/index.ts +++ b/packages/super-editor/src/ui/index.ts @@ -23,6 +23,9 @@ export type { CommentsHandle, CommentsSlice, EqualityFn, + ReviewHandle, + ReviewItem, + ReviewSlice, SelectorFn, SelectionSlice, Subscribable, diff --git a/packages/super-editor/src/ui/review.test.ts b/packages/super-editor/src/ui/review.test.ts new file mode 100644 index 0000000000..0f2a943bfe --- /dev/null +++ b/packages/super-editor/src/ui/review.test.ts @@ -0,0 +1,500 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { createSuperDocUI } from './create-super-doc-ui.js'; +import type { SuperDocLike } from './types.js'; + +/** + * Stub builder for `ui.review` tests. Models the merged feed shape + * — `editor.doc.comments.list()` + `editor.doc.trackChanges.list()` + * + `editor.doc.trackChanges.decide()` + selection routing. + */ +function makeStubs( + initial: { + comments?: Array<{ id: string; commentId: string; text?: string; status?: 'open' | 'resolved' }>; + trackedChanges?: Array<{ + id: string; + type?: 'insert' | 'delete' | 'format'; + excerpt?: string; + story?: unknown; + }>; + activeCommentIds?: string[]; + activeChangeIds?: string[]; + } = {}, +) { + const editorListeners = new Map void>>(); + const superdocListeners = new Map void>>(); + + let commentsList = initial.comments ?? []; + let changesList = initial.trackedChanges ?? []; + + const listComments = vi.fn(() => ({ + evaluatedRevision: 'r1', + total: commentsList.length, + // Mirror the production discovery-item shape: canonical id is on + // `id`, set from the underlying commentId by the adapter. There is + // no `commentId` field on `DiscoveryItem` itself. + items: commentsList.map((c) => ({ + id: c.commentId, + handle: { ref: `comment:${c.commentId}`, refStability: 'stable' as const, targetKind: 'comment' as const }, + address: { kind: 'entity' as const, entityType: 'comment' as const, entityId: c.commentId }, + status: c.status ?? ('open' as const), + text: c.text, + })), + page: { limit: 50, offset: 0, returned: commentsList.length }, + })); + const listChanges = vi.fn((_query?: unknown) => ({ + evaluatedRevision: 'r1', + total: changesList.length, + items: changesList.map((tc) => ({ + id: tc.id, + handle: { + ref: `tracked-change:${tc.id}`, + refStability: 'stable' as const, + targetKind: 'trackedChange' as const, + }, + address: { + kind: 'entity' as const, + entityType: 'trackedChange' as const, + entityId: tc.id, + ...(tc.story != null ? { story: tc.story } : {}), + }, + type: tc.type ?? ('insert' as const), + excerpt: tc.excerpt, + })), + page: { limit: 50, offset: 0, returned: changesList.length }, + })); + const decide = vi.fn((_input: unknown) => ({ success: true as const })); + const scrollIntoView = vi.fn(async (_input: unknown) => ({ success: true as const })); + const setDocumentMode = vi.fn(); + + const editor = { + on: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + if (!editorListeners.has(event)) editorListeners.set(event, new Set()); + editorListeners.get(event)!.add(handler); + }), + off: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + editorListeners.get(event)?.delete(handler); + }), + doc: { + selection: { + current: vi.fn(() => ({ + empty: true, + text: '', + target: null, + activeCommentIds: initial.activeCommentIds ?? [], + activeChangeIds: initial.activeChangeIds ?? [], + })), + }, + comments: { list: listComments, create: vi.fn(), patch: vi.fn(), delete: vi.fn() }, + trackChanges: { list: listChanges, decide }, + ranges: { scrollIntoView }, + }, + }; + + const superdoc: SuperDocLike & { + fireEditor(event: string, ...args: unknown[]): void; + setComments(next: typeof commentsList): void; + setTrackedChanges(next: typeof changesList): void; + setActiveSelection(commentIds?: string[], changeIds?: string[]): void; + } = { + activeEditor: editor as never, + config: { documentMode: 'editing' }, + setDocumentMode: setDocumentMode as never, + on: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + if (!superdocListeners.has(event)) superdocListeners.set(event, new Set()); + superdocListeners.get(event)!.add(handler); + }), + off: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + superdocListeners.get(event)?.delete(handler); + }), + fireEditor(event, ...args) { + const handlers = editorListeners.get(event); + if (!handlers) return; + [...handlers].forEach((handler) => handler(...args)); + }, + setComments(next) { + commentsList = next; + }, + setTrackedChanges(next) { + changesList = next; + }, + setActiveSelection(commentIds = [], changeIds = []) { + (editor.doc.selection.current as unknown as () => unknown) = vi.fn(() => ({ + empty: commentIds.length === 0 && changeIds.length === 0, + text: '', + target: null, + activeCommentIds: commentIds, + activeChangeIds: changeIds, + })); + }, + }; + + return { superdoc, editor, mocks: { listComments, listChanges, decide, scrollIntoView, setDocumentMode } }; +} + +describe('ui.review — snapshot', () => { + it('merges comments and tracked changes into one feed with dense documentOrder', () => { + const { superdoc } = makeStubs({ + comments: [ + { id: 'c1', commentId: 'c1' }, + { id: 'c2', commentId: 'c2' }, + ], + trackedChanges: [ + { id: 'tc1', type: 'insert' }, + { id: 'tc2', type: 'delete' }, + ], + }); + const ui = createSuperDocUI({ superdoc }); + + const snap = ui.review.getSnapshot(); + expect(snap.items).toHaveLength(4); + expect(snap.items.map((i) => ({ kind: i.kind, id: i.id, order: i.documentOrder }))).toEqual([ + { kind: 'comment', id: 'c1', order: 0 }, + { kind: 'comment', id: 'c2', order: 1 }, + { kind: 'change', id: 'tc1', order: 2 }, + { kind: 'change', id: 'tc2', order: 3 }, + ]); + + ui.destroy(); + }); + + it('openCount counts every tracked change + every non-resolved comment', () => { + const { superdoc } = makeStubs({ + comments: [ + { id: 'c1', commentId: 'c1' }, + { id: 'c2', commentId: 'c2', status: 'resolved' }, + { id: 'c3', commentId: 'c3' }, + ], + trackedChanges: [{ id: 'tc1' }, { id: 'tc2' }], + }); + const ui = createSuperDocUI({ superdoc }); + + expect(ui.review.getSnapshot().openCount).toBe(4); // 2 open comments + 2 changes + + ui.destroy(); + }); + + it('activeId mirrors selection.activeCommentIds[0] when on a comment', () => { + const { superdoc } = makeStubs({ + comments: [{ id: 'c1', commentId: 'c1' }], + trackedChanges: [{ id: 'tc1' }], + activeCommentIds: ['c1'], + }); + const ui = createSuperDocUI({ superdoc }); + + expect(ui.review.getSnapshot().activeId).toBe('c1'); + + ui.destroy(); + }); + + it('activeId falls back to selection.activeChangeIds[0] when no active comment', () => { + const { superdoc } = makeStubs({ + comments: [{ id: 'c1', commentId: 'c1' }], + trackedChanges: [{ id: 'tc1' }], + activeChangeIds: ['tc1'], + }); + const ui = createSuperDocUI({ superdoc }); + + expect(ui.review.getSnapshot().activeId).toBe('tc1'); + + ui.destroy(); + }); + + it('subscribe fires once with the initial snapshot', () => { + const { superdoc } = makeStubs({ comments: [{ id: 'c1', commentId: 'c1' }] }); + const ui = createSuperDocUI({ superdoc }); + + const cb = vi.fn(); + const off = ui.review.subscribe(cb); + + expect(cb).toHaveBeenCalledTimes(1); + const arg = cb.mock.calls[0][0] as { snapshot: { items: unknown[] } }; + expect(arg.snapshot.items).toHaveLength(1); + + off(); + ui.destroy(); + }); +}); + +describe('ui.review — decide actions route through editor.doc.trackChanges.*', () => { + it('accept(id) routes to decide({ decision: "accept", target: { id } })', () => { + const { superdoc, mocks } = makeStubs({ trackedChanges: [{ id: 'tc1' }] }); + const ui = createSuperDocUI({ superdoc }); + + ui.review.accept('tc1'); + + expect(mocks.decide).toHaveBeenCalledWith({ decision: 'accept', target: { id: 'tc1' } }); + ui.destroy(); + }); + + it('reject(id) routes to decide({ decision: "reject", target: { id } })', () => { + const { superdoc, mocks } = makeStubs({ trackedChanges: [{ id: 'tc1' }] }); + const ui = createSuperDocUI({ superdoc }); + + ui.review.reject('tc1'); + + expect(mocks.decide).toHaveBeenCalledWith({ decision: 'reject', target: { id: 'tc1' } }); + ui.destroy(); + }); + + it('acceptAll() routes to decide({ scope: "all" })', () => { + const { superdoc, mocks } = makeStubs({ trackedChanges: [{ id: 'tc1' }, { id: 'tc2' }] }); + const ui = createSuperDocUI({ superdoc }); + + ui.review.acceptAll(); + + expect(mocks.decide).toHaveBeenCalledWith({ decision: 'accept', target: { scope: 'all' } }); + ui.destroy(); + }); + + it('rejectAll() routes to decide({ scope: "all" })', () => { + const { superdoc, mocks } = makeStubs({ trackedChanges: [{ id: 'tc1' }, { id: 'tc2' }] }); + const ui = createSuperDocUI({ superdoc }); + + ui.review.rejectAll(); + + expect(mocks.decide).toHaveBeenCalledWith({ decision: 'reject', target: { scope: 'all' } }); + ui.destroy(); + }); +}); + +describe('ui.review — next/previous navigation', () => { + it('next() advances activeId in document order', () => { + const { superdoc } = makeStubs({ + comments: [ + { id: 'c1', commentId: 'c1' }, + { id: 'c2', commentId: 'c2' }, + ], + trackedChanges: [{ id: 'tc1' }], + }); + const ui = createSuperDocUI({ superdoc }); + + expect(ui.review.next()).toBe('c1'); + expect(ui.review.getSnapshot().activeId).toBe('c1'); + + expect(ui.review.next()).toBe('c2'); + expect(ui.review.next()).toBe('tc1'); + }); + + it('next() wraps from the last item to the first', () => { + const { superdoc } = makeStubs({ + comments: [{ id: 'c1', commentId: 'c1' }], + trackedChanges: [{ id: 'tc1' }], + }); + const ui = createSuperDocUI({ superdoc }); + + ui.review.next(); // c1 + ui.review.next(); // tc1 + expect(ui.review.next()).toBe('c1'); // wrap + }); + + it('previous() walks backward and wraps from first to last', () => { + const { superdoc } = makeStubs({ + comments: [{ id: 'c1', commentId: 'c1' }], + trackedChanges: [{ id: 'tc1' }, { id: 'tc2' }], + }); + const ui = createSuperDocUI({ superdoc }); + + expect(ui.review.previous()).toBe('tc2'); // null → wrap to last + expect(ui.review.previous()).toBe('tc1'); + expect(ui.review.previous()).toBe('c1'); + expect(ui.review.previous()).toBe('tc2'); // wrap + }); + + it('next() / previous() return null when the feed is empty', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + expect(ui.review.next()).toBe(null); + expect(ui.review.previous()).toBe(null); + expect(ui.review.getSnapshot().activeId).toBe(null); + + ui.destroy(); + }); +}); + +describe('ui.review — scrollTo + setRecording', () => { + it('scrollTo(id) routes to ranges.scrollIntoView with the right entity type', async () => { + const { superdoc, mocks } = makeStubs({ + comments: [{ id: 'c1', commentId: 'c1' }], + trackedChanges: [{ id: 'tc1' }], + }); + const ui = createSuperDocUI({ superdoc }); + + await ui.review.scrollTo('c1'); + let arg = mocks.scrollIntoView.mock.calls[0][0] as { target: { entityType: string; entityId: string } }; + expect(arg.target).toEqual({ kind: 'entity', entityType: 'comment', entityId: 'c1' }); + + await ui.review.scrollTo('tc1'); + arg = mocks.scrollIntoView.mock.calls[1][0] as { target: { entityType: string; entityId: string } }; + expect(arg.target).toEqual({ kind: 'entity', entityType: 'trackedChange', entityId: 'tc1' }); + + ui.destroy(); + }); + + it('setRecording(true) flips documentMode to suggesting (temporary path)', () => { + const { superdoc, mocks } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + ui.review.setRecording(true); + expect(mocks.setDocumentMode).toHaveBeenCalledWith('suggesting'); + + ui.review.setRecording(false); + expect(mocks.setDocumentMode).toHaveBeenCalledWith('editing'); + + ui.destroy(); + }); +}); + +describe('ui.review — regression: comment row id sourced from discovery.id', () => { + it('comment ReviewItem.id mirrors the discovery item id (not undefined commentId)', () => { + const { superdoc } = makeStubs({ + comments: [ + { id: 'c1', commentId: 'c1' }, + { id: 'c2', commentId: 'c2' }, + ], + }); + const ui = createSuperDocUI({ superdoc }); + + const ids = ui.review.getSnapshot().items.map((i) => i.id); + // Without the fix every comment row would expose `id: undefined` + // because `DiscoveryItem` has no `commentId` field. + expect(ids).toEqual(['c1', 'c2']); + expect(ids.every((id) => typeof id === 'string' && id.length > 0)).toBe(true); + + // And navigation must work on those ids end-to-end. + expect(ui.review.next()).toBe('c1'); + expect(ui.review.next()).toBe('c2'); + + ui.destroy(); + }); +}); + +describe('ui.review — regression: navigation persists past the selected item', () => { + it('next() while the cursor is on the active item is not overwritten by the unchanged selection', async () => { + const { superdoc } = makeStubs({ + comments: [ + { id: 'c1', commentId: 'c1' }, + { id: 'c2', commentId: 'c2' }, + ], + trackedChanges: [{ id: 'tc1' }], + activeCommentIds: ['c1'], + }); + const ui = createSuperDocUI({ superdoc }); + + // Selection lands on c1 → activeId mirrors selection + expect(ui.review.getSnapshot().activeId).toBe('c1'); + + // User clicks "Next" in the sidebar — selection has not moved (still on c1) + expect(ui.review.next()).toBe('c2'); + expect(ui.review.getSnapshot().activeId).toBe('c2'); + + // A subsequent recompute (e.g. typing emits transaction → selectionUpdate) + // must NOT snap activeReviewId back to the selection-driven id, because + // the selection has not moved since the last computeState. + superdoc.fireEditor('selectionUpdate'); + await Promise.resolve(); + expect(ui.review.getSnapshot().activeId).toBe('c2'); + + superdoc.fireEditor('transaction'); + await Promise.resolve(); + expect(ui.review.getSnapshot().activeId).toBe('c2'); + + ui.destroy(); + }); +}); + +describe('ui.review — regression: trackedChangesUpdate refreshes cache', () => { + it('a trackedChangesUpdate event surfaces fresh items in the next snapshot', async () => { + const { superdoc } = makeStubs({ + trackedChanges: [{ id: 'tc1' }], + }); + const ui = createSuperDocUI({ superdoc }); + + expect(ui.review.getSnapshot().items.map((i) => i.id)).toEqual(['tc1']); + + superdoc.setTrackedChanges([{ id: 'tc1' }, { id: 'tc2' }]); + superdoc.fireEditor('trackedChangesUpdate'); + await Promise.resolve(); + + expect(ui.review.getSnapshot().items.map((i) => i.id)).toEqual(['tc1', 'tc2']); + + ui.destroy(); + }); +}); + +describe('ui.review — regression: decide carries non-body story', () => { + it('accept(id) on a header change includes target.story so the adapter routes correctly', () => { + const { superdoc, mocks } = makeStubs({ + trackedChanges: [{ id: 'tc-header', story: 'header:rId1' }], + }); + const ui = createSuperDocUI({ superdoc }); + + ui.review.accept('tc-header'); + + expect(mocks.decide).toHaveBeenCalledWith({ + decision: 'accept', + target: { id: 'tc-header', story: 'header:rId1' }, + }); + + ui.destroy(); + }); + + it('reject(id) on a footer change includes target.story', () => { + const { superdoc, mocks } = makeStubs({ + trackedChanges: [{ id: 'tc-footer', story: 'footer:rId2' }], + }); + const ui = createSuperDocUI({ superdoc }); + + ui.review.reject('tc-footer'); + + expect(mocks.decide).toHaveBeenCalledWith({ + decision: 'reject', + target: { id: 'tc-footer', story: 'footer:rId2' }, + }); + + ui.destroy(); + }); + + it('accept(id) on a body change omits target.story (parity with body-default contract)', () => { + const { superdoc, mocks } = makeStubs({ + trackedChanges: [{ id: 'tc-body' }], + }); + const ui = createSuperDocUI({ superdoc }); + + ui.review.accept('tc-body'); + + expect(mocks.decide).toHaveBeenCalledWith({ + decision: 'accept', + target: { id: 'tc-body' }, + }); + + 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({ + comments: [{ id: 'c1', commentId: 'c1' }], + trackedChanges: [{ id: 'tc1' }], + }); + const ui = createSuperDocUI({ superdoc }); + + const cb = vi.fn(); + const off = ui.review.subscribe(cb); + expect(cb).toHaveBeenCalledTimes(1); // initial snapshot + + superdoc.fireEditor('transaction'); + await Promise.resolve(); + superdoc.fireEditor('selectionUpdate'); + await Promise.resolve(); + + // Memoization keeps the slice identity-stable when the source caches and + // activeReviewId have not changed, so shallowEqual short-circuits. + expect(cb).toHaveBeenCalledTimes(1); + + off(); + ui.destroy(); + }); +}); diff --git a/packages/super-editor/src/ui/types.ts b/packages/super-editor/src/ui/types.ts index 08089ea1f9..b7267d8993 100644 --- a/packages/super-editor/src/ui/types.ts +++ b/packages/super-editor/src/ui/types.ts @@ -45,6 +45,12 @@ export interface SuperDocLike { off?(event: string, handler: (...args: unknown[]) => void): unknown; activeEditor?: SuperDocEditorLike | null; config?: { documentMode?: 'editing' | 'suggesting' | 'viewing' }; + /** + * Optional setter for documentMode. Used by `ui.review.setRecording` + * as the temporary path until S4 ships an independent + * `trackChanges.setRecording` primitive. + */ + setDocumentMode?(mode: 'editing' | 'suggesting' | 'viewing'): unknown; } export interface SuperDocEditorLike { @@ -77,6 +83,14 @@ export interface SuperDocEditorLike { ranges?: { scrollIntoView?(input: unknown): Promise; }; + /** + * Tracked-changes member on the Document API. Used by + * `ui.review.*` for accept/reject and the merged feed. + */ + trackChanges?: { + list?(query?: unknown): unknown; + decide?(input: unknown, options?: unknown): unknown; + }; }; } @@ -119,6 +133,13 @@ export interface SuperDocUIState { * can highlight the active card without a separate subscription. */ comments: CommentsSlice; + /** + * Review slice — merged comments + tracked-changes feed for the + * Word / Google Docs review sidebar pattern. Cached at controller + * level alongside the comments slice; refreshes on the same events + * plus tracked-change events. + */ + review: ReviewSlice; } /** @@ -156,6 +177,59 @@ export interface CommentsSlice { activeIds: string[]; } +/** + * One item in the merged review feed (comments + tracked changes). + * + * Discriminated by `kind`. `documentOrder` is a dense rank within the + * snapshot — comparing two items' `documentOrder` tells you which + * appears first; consuming UIs don't need to recompute it. + */ +export type ReviewItem = + | { + kind: 'comment'; + id: string; + documentOrder: number; + comment: import('@superdoc/document-api').CommentsListResult['items'][number]; + } + | { + kind: 'change'; + id: string; + documentOrder: number; + change: import('@superdoc/document-api').TrackChangesListResult['items'][number]; + }; + +/** + * Snapshot of the merged review feed exposed on `state.review`. + * + * Document-order ranking note (per SD-2791 ticket): both + * `editor.doc.trackChanges.list()` and tracked-change groupings are + * already returned in PM-position order, but cross-list interleaving + * between comments and tracked changes is *not* fully resolved + * because public `TrackChangeInfo` lacks a positional `target` today + * (separate ticket). The initial implementation interleaves comments + * (in their `comments.list()` order) ahead of tracked changes (in + * their `list()` order); migration-guide consumers get a stable + * iteration order and dense `documentOrder` ranks for next/previous + * navigation. When `TrackChangeInfo.target` lands, the merge sort + * gets refined transparently. + */ +export interface ReviewSlice { + /** Merged feed, sorted by `documentOrder`. */ + items: ReviewItem[]; + /** + * Number of unresolved review items (open comments + every tracked + * change). Drives sidebar-header counts. + */ + openCount: number; + /** + * The currently active item id — driven by selection + * (`activeCommentIds[0] ?? activeChangeIds[0]`) plus + * `ui.review.next/previous/scrollTo` calls. `null` when nothing is + * focused. + */ + activeId: string | null; +} + export interface SuperDocUIOptions { superdoc: SuperDocLike; } @@ -172,6 +246,7 @@ export interface SuperDocUI { */ select(selector: SelectorFn, equality?: EqualityFn): Subscribable; + /** * Aggregate toolbar surface. Mirrors the `HeadlessToolbarController` * shape from `superdoc/headless-toolbar`, sourced from the same * internal controller. Equivalent to subscribing to the toolbar slice @@ -199,6 +274,13 @@ export interface SuperDocUI { */ comments: CommentsHandle; + /** + * Review domain — merged comments + tracked-changes feed for + * Word/Google-Docs review sidebars. Same shape as `comments` but + * with accept/reject/next/previous semantics. + */ + review: ReviewHandle; + /** * Tear down all internal subscriptions to the editor / SuperDoc * instance / presentation editor. After destroy, no listeners will @@ -311,3 +393,55 @@ export interface CommentsHandle { */ scrollTo(commentId: string): Promise; } + +/** + * Review domain handle exposed on `ui.review`. Same architectural + * posture as `CommentsHandle`: every mutation routes through + * `editor.doc.trackChanges.*` (the Document API contract); next / + * previous / scrollTo are UI-only navigation helpers. + */ +export interface ReviewHandle { + /** Snapshot the merged review feed synchronously. */ + getSnapshot(): ReviewSlice; + /** + * Subscribe to review-snapshot changes (items, openCount, activeId). + * Listener fires once synchronously with the current snapshot, then + * again whenever the slice changes by shallow equality. Returns an + * unsubscribe. + */ + subscribe(listener: (event: { snapshot: ReviewSlice }) => void): () => void; + /** Accept a single tracked change via `trackChanges.decide`. */ + accept(changeId: string): import('@superdoc/document-api').Receipt; + /** Reject a single tracked change via `trackChanges.decide`. */ + reject(changeId: string): import('@superdoc/document-api').Receipt; + /** Accept every tracked change via `trackChanges.decide({ scope: 'all' })`. */ + acceptAll(): import('@superdoc/document-api').Receipt; + /** Reject every tracked change via `trackChanges.decide({ scope: 'all' })`. */ + rejectAll(): import('@superdoc/document-api').Receipt; + /** + * Move `activeId` to the next item in the merged feed (document + * order). Wraps to the first item past the last. Returns the new + * active id, or `null` if the feed is empty. + */ + next(): string | null; + /** + * Move `activeId` to the previous item in the merged feed. Wraps + * to the last item past the first. Returns the new active id, or + * `null` if the feed is empty. + */ + previous(): string | null; + /** + * Scroll the viewport to the given item (comment or tracked + * change) and set it as `activeId`. Routes through + * `editor.doc.ranges.scrollIntoView({ target: EntityAddress })`. + */ + scrollTo(id: string): Promise; + /** + * Toggle tracked-changes recording. Today flips + * `superdoc.config.documentMode` between `'suggesting'` and + * `'editing'`; SD-2667's S4 follow-up will decouple recording from + * view mode and this routes through the new primitive once + * available. + */ + setRecording(enabled: boolean): void; +} diff --git a/packages/superdoc/src/ui.d.ts b/packages/superdoc/src/ui.d.ts index b99d665d7b..f1aa5e4923 100644 --- a/packages/superdoc/src/ui.d.ts +++ b/packages/superdoc/src/ui.d.ts @@ -4,6 +4,9 @@ export { type CommentsHandle, type CommentsSlice, type EqualityFn, + type ReviewHandle, + type ReviewItem, + type ReviewSlice, type SelectorFn, type SelectionSlice, type Subscribable,