From 139765cf1f5ae88670f0a1e220243a880e1be3e4 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Fri, 29 May 2026 15:58:15 -0300 Subject: [PATCH] feat(super-editor): add ui.comments.setActive for activate-only comment highlight (SD-3319) Adds an activate-only method to the Custom UI comments handle so a consumer that scrolls a comment into view itself (e.g. ui.viewport rect plus its own scroll) can mark it active without using scrollTo. Routes through the existing setActiveComment command: highlight-only, no scroll, no selection move, and getSnapshot().activeIds stays selection-derived. Unknown ids (by id or importedId) are rejected so activating a missing id can't fade every other comment. --- packages/super-editor/src/ui/comments.test.ts | 128 +++++++++++++++++- .../src/ui/create-super-doc-ui.ts | 29 ++++ packages/super-editor/src/ui/types.ts | 36 ++++- 3 files changed, 188 insertions(+), 5 deletions(-) diff --git a/packages/super-editor/src/ui/comments.test.ts b/packages/super-editor/src/ui/comments.test.ts index 516dfe9b3b..dd84aee4c5 100644 --- a/packages/super-editor/src/ui/comments.test.ts +++ b/packages/super-editor/src/ui/comments.test.ts @@ -10,7 +10,13 @@ import type { SuperDocLike } from './types.js'; */ function makeStubs( initial: { - comments?: Array<{ id: string; commentId: string; text?: string; status?: 'open' | 'resolved' }>; + comments?: Array<{ + id: string; + commentId: string; + importedId?: string; + text?: string; + status?: 'open' | 'resolved'; + }>; activeCommentIds?: string[]; selectionTarget?: unknown; } = {}, @@ -37,17 +43,21 @@ function makeStubs( 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 }, commentId: c.commentId, + importedId: c.importedId, status: c.status ?? ('open' as const), text: c.text, })), page: { limit: 50, offset: 0, returned: commentsList.length }, })); const navigateTo = vi.fn(async (_target: unknown) => true); + const setActiveComment = vi.fn((_input: { commentId: string | null }) => true); + const setCursorById = vi.fn((_id: string, _options?: unknown) => true); const editor: { on: ReturnType; off: ReturnType; doc: unknown; + commands: { setActiveComment: typeof setActiveComment; setCursorById: typeof setCursorById }; presentationEditor: { navigateTo: typeof navigateTo; getActiveEditor: () => unknown; @@ -72,6 +82,7 @@ function makeStubs( }, comments: { create, patch, delete: del, list }, }, + commands: { setActiveComment, setCursorById }, // Self-reference assigned below so toolbar source resolution sees // the same routed editor as the rest of the stub. presentationEditor: undefined as never, @@ -101,7 +112,11 @@ function makeStubs( }, }; - return { superdoc, editor, mocks: { create, patch, delete: del, list, navigateTo } }; + return { + superdoc, + editor, + mocks: { create, patch, delete: del, list, navigateTo, setActiveComment, setCursorById }, + }; } const flushMicrotasks = () => Promise.resolve(); @@ -495,3 +510,112 @@ describe('ui.comments — actions route through editor.doc.*', () => { ui.destroy(); }); }); + +describe('ui.comments.setActive — activate-only highlight', () => { + it('routes a known comment id through the setActiveComment command', () => { + const { superdoc, mocks } = makeStubs({ comments: [{ id: 'c-1', commentId: 'c-1' }] }); + const ui = createSuperDocUI({ superdoc }); + + const ok = ui.comments.setActive('c-1'); + + expect(ok).toBe(true); + expect(mocks.setActiveComment).toHaveBeenCalledTimes(1); + expect(mocks.setActiveComment).toHaveBeenCalledWith({ commentId: 'c-1' }); + + ui.destroy(); + }); + + it('clears the active highlight when passed null (no id validation)', () => { + const { superdoc, mocks } = makeStubs({ comments: [{ id: 'c-1', commentId: 'c-1' }] }); + const ui = createSuperDocUI({ superdoc }); + + const ok = ui.comments.setActive(null); + + expect(ok).toBe(true); + expect(mocks.setActiveComment).toHaveBeenCalledWith({ commentId: null }); + + ui.destroy(); + }); + + it('does not scroll or move the selection (no navigateTo, no setCursorById)', () => { + const { superdoc, mocks } = makeStubs({ comments: [{ id: 'c-1', commentId: 'c-1' }] }); + const ui = createSuperDocUI({ superdoc }); + + ui.comments.setActive('c-1'); + + expect(mocks.navigateTo).not.toHaveBeenCalled(); + expect(mocks.setCursorById).not.toHaveBeenCalled(); + + ui.destroy(); + }); + + it('returns false for an unknown id and does not dispatch (avoids fading every comment)', () => { + const { superdoc, mocks } = makeStubs({ comments: [{ id: 'c-1', commentId: 'c-1' }] }); + const ui = createSuperDocUI({ superdoc }); + + const ok = ui.comments.setActive('does-not-exist'); + + expect(ok).toBe(false); + expect(mocks.setActiveComment).not.toHaveBeenCalled(); + + ui.destroy(); + }); + + it('validates against the imported Word id, not just the native id', () => { + // Imported Word comments carry a separate `importedId` that the + // highlight painter also keys on; consumers loading from .docx + // activate by that id, so it must be accepted. + const { superdoc, mocks } = makeStubs({ + comments: [{ id: 'native-1', commentId: 'native-1', importedId: 'imported-abc' }], + }); + const ui = createSuperDocUI({ superdoc }); + + const ok = ui.comments.setActive('imported-abc'); + + expect(ok).toBe(true); + expect(mocks.setActiveComment).toHaveBeenCalledWith({ commentId: 'imported-abc' }); + + ui.destroy(); + }); + + it('passes through a false result from the underlying command', () => { + const { superdoc, mocks } = makeStubs({ comments: [{ id: 'c-1', commentId: 'c-1' }] }); + mocks.setActiveComment.mockReturnValueOnce(false); + const ui = createSuperDocUI({ superdoc }); + + const ok = ui.comments.setActive('c-1'); + + expect(mocks.setActiveComment).toHaveBeenCalledWith({ commentId: 'c-1' }); + expect(ok).toBe(false); + + ui.destroy(); + }); + + it('does not change getSnapshot().activeIds (the slice stays selection-derived)', () => { + const { superdoc } = makeStubs({ comments: [{ id: 'c-1', commentId: 'c-1' }], activeCommentIds: [] }); + const ui = createSuperDocUI({ superdoc }); + + const before = ui.comments.getSnapshot().activeIds; + ui.comments.setActive('c-1'); + const after = ui.comments.getSnapshot().activeIds; + + expect(after).toEqual(before); + expect(after).toEqual([]); + + ui.destroy(); + }); + + it('returns false when no editor is mounted', () => { + const superdoc = { + activeEditor: null, + config: { documentMode: 'editing' }, + on: vi.fn(), + off: vi.fn(), + } as unknown as SuperDocLike; + const ui = createSuperDocUI({ superdoc }); + + expect(ui.comments.setActive('c-1')).toBe(false); + + ui.destroy(); + }); +}); 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 3d61f0b670..5a93c69758 100644 --- a/packages/super-editor/src/ui/create-super-doc-ui.ts +++ b/packages/super-editor/src/ui/create-super-doc-ui.ts @@ -1767,6 +1767,35 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { refreshAndNotify(); return receipt; }, + setActive(commentId) { + // Dispatch on the host editor: its `PresentationEditor` is what + // repaints comment highlights (it listens to the host editor's + // `commentsUpdate`), so the activation must originate there. + const editor = resolveHostEditor(superdoc) as unknown as { + commands?: { setActiveComment?(input: { commentId: string | null }): boolean }; + doc?: { comments?: { list?(): { items?: ReadonlyArray<{ id?: string; importedId?: string }> } } }; + } | null; + const setActiveComment = editor?.commands?.setActiveComment; + if (typeof setActiveComment !== 'function') return false; + + if (commentId !== null) { + // Guard unknown ids. When an id is active the painter fades + // every *other* comment; an id that matches nothing would dim + // the whole document with no highlight shown. Accept only ids + // present in the current comment list — matching by `id` or the + // imported Word id, both of which the painter keys on. + const items = editor?.doc?.comments?.list?.().items ?? []; + const known = items.some((c) => c.id === commentId || c.importedId === commentId); + if (!known) return false; + } + + // Highlight-only. `setActiveComment` sets plugin meta the painter + // repaints from; it does not scroll, move the selection, or focus + // the editor (unlike `setCursorById`, which `scrollTo` uses). + // `activeIds` is selection-derived and intentionally left + // untouched, so no `refreshAndNotify()` here. + return setActiveComment({ commentId }) === true; + }, async scrollTo(commentId) { // `CommentAddress` is body-scoped in the contract — it has no // `story` field today. Story-aware comment navigation lands as diff --git a/packages/super-editor/src/ui/types.ts b/packages/super-editor/src/ui/types.ts index 3517136ce2..b75a754b41 100644 --- a/packages/super-editor/src/ui/types.ts +++ b/packages/super-editor/src/ui/types.ts @@ -1634,9 +1634,13 @@ export type CustomCommandHandleState = { }; /** - * Comments domain handle exposed on `ui.comments`. The execute - * methods are convenience facades over `editor.doc.comments.*` — - * they produce identical document mutations to direct doc-API calls. + * Comments domain handle exposed on `ui.comments`. The mutation + * methods (`createFromSelection`, `createFromCapture`, `reply`, + * `resolve`, `reopen`, `delete`) are convenience facades over + * `editor.doc.comments.*` — they produce identical document mutations + * to direct doc-API calls. `setActive` and `scrollTo` are UI-only + * helpers: they drive highlight / viewport state and do not mutate the + * document. */ export interface CommentsHandle { /** Snapshot the current comments slice synchronously. */ @@ -1698,6 +1702,32 @@ export interface CommentsHandle { reopen(commentId: string): import('@superdoc/document-api').Receipt; /** Delete a comment via `editor.doc.comments.delete`. */ delete(commentId: string): import('@superdoc/document-api').Receipt; + /** + * Activate (or clear) a comment's highlight in the document without + * scrolling or moving the selection. Pass a comment id to highlight + * it as active; pass `null` to clear the active highlight. Routes + * through the internal `setActiveComment` command, which the + * presentation layer repaints from. + * + * This is the activate-only counterpart to {@link scrollTo}: use it + * when you bring the comment into view yourself (e.g. `ui.viewport` + * rect + your own scroll) and only need to mark it active afterwards. + * Unlike `scrollTo`, it does not move the caret or scroll. + * + * It also does not change `getSnapshot().activeIds` — that set stays + * selection-derived, so the document highlight and the snapshot's + * active set are intentionally decoupled. (A future `focusedId` on + * the slice could expose this to subscribers if a consumer needs it.) + * + * Returns `true` when the activation request was accepted and + * dispatched — including an idempotent call for an id that is already + * active. Returns `false` when no editor is mounted, when the + * underlying command reports failure, or when a non-null `commentId` + * matches no current comment — activating an unknown id would + * otherwise fade every other comment as if some other comment were + * active. + */ + setActive(commentId: string | null): boolean; /** * Scroll the viewport to the comment's anchor via * `ui.viewport.scrollIntoView({ target: EntityAddress })`. Resolves