fix(comments): prevent flicker when clicking comment with tracked changes

packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js

@@ -507,11 +507,10 @@ export const CommentsPlugin = Extension.create({
           }
 
           // Check for changes in the actively selected comment
-          const trChangedActiveComment = meta?.type === 'setActiveComment';
-          if ((!tr.docChanged && tr.selectionSet) || trChangedActiveComment) {
+          if (!tr.docChanged && tr.selectionSet) {
             const { selection } = tr;
+
             let currentActiveThread = getActiveCommentId(newEditorState.doc, selection);
-            if (trChangedActiveComment) currentActiveThread = meta.activeThreadId;
             if (
               meta?.type === 'setCursorById' &&
               meta.preferredActiveThreadId &&
@@ -521,7 +520,20 @@ export const CommentsPlugin = Extension.create({
             }
 
             const previousSelectionId = pluginState.activeThreadId;
-            if (previousSelectionId !== currentActiveThread) {
+            // getActiveCommentId returns undefined for any non-collapsed selection
+            // (its first line is `if ($from.pos !== $to.pos) return;`), and that
+            // undefined gets coerced to null below, which would emit
+            // commentsUpdate({activeCommentId: null}) and clear an active comment.
+            // For tracked-change comments, presentation.navigateTo dispatches a
+            // collapsed cursor placement immediately followed by a non-collapsed
+            // NodeSelection on the SDT wrapper. That second tx would otherwise
+            // overwrite the just-activated comment, the host's commentsUpdate
+            // listener would re-assert it, and the loop would flicker forever
+            // (issue #2861). Treat "undefined from non-collapsed" as "no
+            // information" rather than "no active comment".
+            const isNonCollapsedClear =
+              currentActiveThread == null && selection && selection.$from.pos !== selection.$to.pos;
+            if (previousSelectionId !== currentActiveThread && !isNonCollapsedClear) {
               // Update both the plugin state and the local variable
               pluginState.activeThreadId = currentActiveThread;
               const update = {

packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js

@@ -1641,6 +1641,149 @@ describe('SD-1940: no recursive dispatch from apply() on selection change', () =
   });
 });
 
+// Issue #2861: clicking a comment whose range overlaps a tracked change inside an inline
+// SDT triggers `presentation.navigateTo`, which dispatches a collapsed cursor placement
+// followed by a non-collapsed NodeSelection on the SDT wrapper. `getActiveCommentId` early-
+// returns `undefined` for any non-collapsed selection, and the plugin used to coerce that
+// `undefined` into `commentsUpdate({activeCommentId: null})`, clearing the just-activated
+// comment. The host's `commentsUpdate` listener then re-asserts the comment, the plugin
+// re-emits, and the highlight class flickers ~400 times/second until something else changes.
+describe('SD-2861: non-collapsed selection does not clear active comment', () => {
+  it('preserves activeThreadId when a non-collapsed selection follows explicit activation', () => {
+    const schema = createCommentSchema();
+    const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true });
+    // "Hello" (1..6) carries the comment, " World" (6..12) does not.
+    const paragraph = schema.node('paragraph', null, [schema.text('Hello', [commentMark]), schema.text(' World')]);
+    const doc = schema.node('doc', null, [paragraph]);
+    const { view, editor } = createPluginStateEnvironment({ schema, doc });
+
+    // Step 1: Explicitly activate the comment (mirrors the sidebar click path).
+    view.dispatch(
+      view.state.tr.setMeta(CommentsPluginKey, {
+        type: 'setActiveComment',
+        activeThreadId: 'thread-1',
+        forceUpdate: true,
+      }),
+    );
+    expect(CommentsPluginKey.getState(view.state).activeThreadId).toBe('thread-1');
+    editor.emit.mockClear();
+
+    // Step 2: Non-collapsed selection that straddles the comment boundary, mimicking the
+    // NodeSelection that `presentation.navigateTo` produces on the SDT wrapper.
+    view.dispatch(view.state.tr.setSelection(TextSelection.create(view.state.doc, 1, 8)));
+
+    // The active comment must survive. `getActiveCommentId` returned undefined (because the
+    // selection was non-collapsed); the plugin used to treat that as "no comment" and emit
+    // commentsUpdate({activeCommentId: null}). It must not.
+    expect(CommentsPluginKey.getState(view.state).activeThreadId).toBe('thread-1');
+    expect(editor.emit).not.toHaveBeenCalledWith('commentsUpdate', expect.objectContaining({ activeCommentId: null }));
+  });
+
+  it('preserves activeThreadId when a follow-up non-collapsed selection sits entirely within the comment range', () => {
+    const schema = createCommentSchema();
+    const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true });
+    const trackedMark = schema.marks[TrackInsertMarkName].create({ id: 'tc-1' });
+    // The repro case from #2861: text carries both a comment mark and a tracked-change mark.
+    const paragraph = schema.node('paragraph', null, [schema.text('WYSIWYG', [commentMark, trackedMark])]);
+    const doc = schema.node('doc', null, [paragraph]);
+    const { view, editor } = createPluginStateEnvironment({ schema, doc });
+
+    view.dispatch(
+      view.state.tr.setMeta(CommentsPluginKey, {
+        type: 'setActiveComment',
+        activeThreadId: 'thread-1',
+        forceUpdate: true,
+      }),
+    );
+    editor.emit.mockClear();
+
+    // Non-collapsed selection that wraps the entire comment range — what navigateTo
+    // produces when it lands a NodeSelection on the SDT containing the tracked change.
+    view.dispatch(view.state.tr.setSelection(TextSelection.create(view.state.doc, 1, 8)));
+
+    expect(CommentsPluginKey.getState(view.state).activeThreadId).toBe('thread-1');
+    expect(editor.emit).not.toHaveBeenCalledWith('commentsUpdate', expect.objectContaining({ activeCommentId: null }));
+  });
+
+  it('still clears activeThreadId when a collapsed cursor moves outside the comment range', () => {
+    // Regression guard: the fix only suppresses clears for non-collapsed selections.
+    // A real "user moved off the comment" interaction (collapsed cursor outside any comment)
+    // must still propagate.
+    const schema = createCommentSchema();
+    const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true });
+    const paragraph = schema.node('paragraph', null, [schema.text('Hello', [commentMark]), schema.text(' World')]);
+    const doc = schema.node('doc', null, [paragraph]);
+    const { view, editor } = createPluginStateEnvironment({ schema, doc });
+
+    view.dispatch(
+      view.state.tr.setMeta(CommentsPluginKey, {
+        type: 'setActiveComment',
+        activeThreadId: 'thread-1',
+        forceUpdate: true,
+      }),
+    );
+    editor.emit.mockClear();
+
+    // Collapsed cursor at pos 9 lands inside " World", which has no comment mark.
+    view.dispatch(view.state.tr.setSelection(TextSelection.create(view.state.doc, 9)));
+
+    expect(CommentsPluginKey.getState(view.state).activeThreadId).toBeNull();
+    expect(editor.emit).toHaveBeenCalledWith('commentsUpdate', expect.objectContaining({ activeCommentId: null }));
+  });
+
+  it('does not enter a dispatch loop when a host listener re-asserts after each commentsUpdate', () => {
+    // Models the live system: SuperDoc.vue's onEditorCommentsUpdate calls
+    // commentsStore.setActiveComment(superdoc, payload.activeCommentId), which dispatches
+    // another setActiveComment meta back into the editor. Before the fix, the cursor + range
+    // tx pair from navigateTo emitted (id, null) which the listener echoed back, looping.
+    const schema = createCommentSchema();
+    const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true });
+    const paragraph = schema.node('paragraph', null, [schema.text('Hello', [commentMark])]);
+    const doc = schema.node('doc', null, [paragraph]);
+
+    const { editor, view, extension } = createEditorEnvironment(schema, doc);
+    const plugins = extension.addPmPlugins();
+    view.state = EditorState.create({ schema, doc, selection: TextSelection.create(doc, 1), plugins });
+
+    let dispatchCount = 0;
+    const originalDispatch = (tr) => {
+      view.state = view.state.apply(tr);
+    };
+    view.dispatch = vi.fn((tr) => {
+      dispatchCount += 1;
+      if (dispatchCount > 10) throw new Error('Dispatch loop detected — exceeded 10 dispatches');
+      originalDispatch(tr);
+    });
+
+    // Mirror the host's reaction: every commentsUpdate triggers a setActiveComment meta back
+    // into the editor. The store does this even when the payload's activeCommentId === null.
+    editor.emit = vi.fn((eventName, payload) => {
+      if (eventName !== 'commentsUpdate') return;
+      view.dispatch(
+        view.state.tr.setMeta(CommentsPluginKey, {
+          type: 'setActiveComment',
+          activeThreadId: payload?.activeCommentId ?? null,
+          forceUpdate: true,
+        }),
+      );
+    });
+
+    // Tx 1: cursor placement at pos 3 inside the comment.
+    view.dispatch(
+      view.state.tr
+        .setSelection(TextSelection.create(view.state.doc, 3))
+        .setMeta(CommentsPluginKey, { type: 'setCursorById', preferredActiveThreadId: 'thread-1' }),
+    );
+    // Tx 2: non-collapsed selection — the SDT-wrapper case from navigateTo.
+    view.dispatch(view.state.tr.setSelection(TextSelection.create(view.state.doc, 1, 6)));
+
+    // Without the fix, dispatchCount would explode (each clear emit triggers another
+    // setActiveComment dispatch). The non-collapsed-clear suppression keeps it bounded.
+    expect(dispatchCount).toBeLessThanOrEqual(5);
+    expect(CommentsPluginKey.getState(view.state).activeThreadId).toBe('thread-1');
+  });
+});
+
 describe('Headless mode plugin behavior', () => {
   it('creates a state-only plugin in headless mode (no props or view)', () => {
     const editor = {

tests/behavior/tests/comments/comment-on-tracked-change.spec.ts

@@ -147,3 +147,115 @@ test('switching highlighted threads does not trigger a second delayed floating-s
   await expect(targetDialog.locator('.comment-body .comment').nth(0)).toContainText('abc');
   await expect(targetDialog.locator('.comment-body .comment').nth(1)).toContainText('xyz');
 });
+
+// SD-2861 regression: explicit comment activation followed by a non-collapsed selection
+// (the shape `presentation.navigateTo` produces when landing a NodeSelection on the SDT
+// wrapper around a tracked change) must not enter a feedback loop. The plugin used to
+// coerce `getActiveCommentId`'s `undefined` return for non-collapsed selections into
+// `commentsUpdate({activeCommentId: null})`. The Vue host re-asserted the comment, the
+// plugin re-emitted, and `.track-change-focused` toggled ~400 times/second.
+//
+// This test programmatically reproduces the two-transaction pattern at the live editor
+// level so it covers the integrated path (plugin -> PresentationEditor bridge -> store
+// listener -> commands.setActiveComment) without depending on a fixture that happens to
+// have an SDT-wrapped tracked change.
+test('explicit comment activation survives a follow-up non-collapsed selection', async ({ superdoc }) => {
+  await superdoc.loadDocument(DOC_PATH);
+  await superdoc.page.waitForSelector('.superdoc-comment-highlight', { timeout: 30_000 });
+  await superdoc.waitForStable();
+  await assertDocumentApiReady(superdoc.page);
+
+  const result = await superdoc.page.evaluate(async () => {
+    const editor = (window as any).editor;
+    const view = editor?.view;
+    if (!view?.dispatch) throw new Error('editor.view.dispatch not available');
+
+    // Find the first comment-marked range in the doc and capture its id + bounds.
+    const commentInfo = ((): { id: string; from: number; to: number } | null => {
+      let id: string | null = null;
+      let from = -1;
+      let to = -1;
+      view.state.doc.descendants((node: any, pos: number) => {
+        for (const mark of node.marks ?? []) {
+          if (mark.type.name !== 'commentMark') continue;
+          const candidateId = mark.attrs?.commentId ?? mark.attrs?.importedId;
+          if (!candidateId) continue;
+          if (id === null) {
+            id = candidateId;
+            from = pos;
+            to = pos + node.nodeSize;
+          } else if (candidateId === id) {
+            to = Math.max(to, pos + node.nodeSize);
+          }
+        }
+      });
+      return id !== null ? { id, from, to } : null;
+    })();
+    if (!commentInfo) throw new Error('No comment-marked range found in fixture');
+
+    let dispatchCount = 0;
+    const originalDispatch = view.dispatch.bind(view);
+    view.dispatch = (tr: unknown) => {
+      dispatchCount += 1;
+      return originalDispatch(tr);
+    };
+
+    let toggles = 0;
+    const observer = new MutationObserver((muts) => {
+      muts.forEach((m) => {
+        if (m.attributeName !== 'class') return;
+        const oldVal = String(m.oldValue ?? '');
+        const newVal = String((m.target as Element).className ?? '');
+        if (oldVal === newVal) return;
+        const before = oldVal.includes('track-change-focused');
+        const after = newVal.includes('track-change-focused');
+        if (before !== after) toggles += 1;
+      });
+    });
+    const pages = document.querySelector('.presentation-editor__pages');
+    if (pages) {
+      observer.observe(pages, {
+        attributes: true,
+        attributeOldValue: true,
+        subtree: true,
+        attributeFilter: ['class'],
+      });
+    }
+
+    try {
+      // Tx 1: explicit activation, mirrors the sidebar-click path.
+      editor.commands.setActiveComment({ commentId: commentInfo.id });
+
+      // Tx 2: non-collapsed selection that wraps the comment range, mirrors the
+      // NodeSelection from `presentation.navigateTo` on an SDT wrapper.
+      const SelectionCtor = view.state.selection.constructor as any;
+      view.dispatch(view.state.tr.setSelection(SelectionCtor.create(view.state.doc, commentInfo.from, commentInfo.to)));
+
+      // Sample for 800ms. With the bug, the (id, null) emit pair plus the host re-assert
+      // produces 200+ toggles/sec; the fix keeps it bounded.
+      await new Promise((r) => setTimeout(r, 800));
+    } finally {
+      observer.disconnect();
+      view.dispatch = originalDispatch;
+    }
+
+    const activePluginState = view.state.plugins
+      .map((p: any) => p.getState?.(view.state))
+      .find((s: any) => s && 'activeThreadId' in s);
+
+    return {
+      dispatchCount,
+      toggles,
+      finalActiveThreadId: activePluginState?.activeThreadId ?? null,
+      expectedActiveCommentId: commentInfo.id,
+    };
+  });
+
+  await superdoc.waitForStable();
+
+  // Without the fix: dispatchCount climbs into the dozens (the host re-asserts on every
+  // commentsUpdate emit) and toggles climbs into the hundreds. With the fix: bounded.
+  expect(result.dispatchCount).toBeLessThanOrEqual(15);
+  expect(result.toggles).toBeLessThanOrEqual(3);
+  expect(result.finalActiveThreadId).toBe(result.expectedActiveCommentId);
+});