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

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

Add explicitlySetThreadId flag to CommentsPlugin state to prevent
position-based active comment detection from overriding sidebar-initiated
comment activation. When a comment is explicitly activated (e.g., sidebar
click), subsequent DOM-sync selection transactions preserve the active
comment as long as the cursor remains within its range.

The flag clears naturally when the cursor moves outside the comment range
or when the document is edited, allowing position-based detection to
resume for normal user navigation.

Closes #2861

* fix(comments): preserve active comment across non-collapsed selection (#2861)

getActiveCommentId returns undefined for any non-collapsed selection. The
plugin coerced that into commentsUpdate({activeCommentId: null}), which the
host listener echoed back as a fresh setActiveComment dispatch. For tracked
changes inside an inline SDT, presentation.navigateTo dispatches a collapsed
cursor placement followed by a non-collapsed NodeSelection on the SDT
wrapper, so the second tx blanked the just-activated comment and triggered a
flicker loop (~400 toggles/sec on .track-change-focused).

Skip the emit when getActiveCommentId returned no information for a
non-collapsed selection. A non-collapsed selection is not a reliable signal
that the user moved off the comment.

* test(comments): unit tests for SD-2861 non-collapsed clear regression

Four tests pinning the new contract:
- preserves activeThreadId when a non-collapsed selection follows explicit
  activation
- preserves activeThreadId when a follow-up non-collapsed selection sits
  entirely within the comment range (the WYSIWYG repro shape)
- still clears activeThreadId when a collapsed cursor moves outside the
  comment range (existing behavior preserved)
- does not enter a dispatch loop when a host listener re-asserts after each
  commentsUpdate (models the SuperDoc.vue store round-trip)

Three of the four fail without the plugin fix.

* test(behavior): comment activation survives non-collapsed selection (#2861)

Programmatically reproduces the two-transaction pattern from
presentation.navigateTo at the live editor level: setActiveComment on a
comment-marked range, then a non-collapsed selection over the same range.
Counts dispatches and class toggles on .track-change-focused.

Without the plugin fix, dispatchCount climbs into the hundreds within a
second as the host re-asserts on every commentsUpdate emit. With the fix,
both counts stay bounded and the active comment survives.

* fix(comments): write explicitlySetThreadId on copy, not original state

Address review feedback — avoid mutating pluginState directly before
spreading. Use conditional copy pattern consistent with Caio's suggestion.

* refactor(comments): remove explicitlySetThreadId flag per review

Drop the flag in favor of Caio's isNonCollapsedClear guard, which
handles the actual flicker trigger (range selection from SDT wrapper).
Keeps the dead code removal and condition simplification from the
original commit.

---------

Co-authored-by: Caio Pizzol <caio@superdoc.dev>

Author: gpardhivvarma

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);
+});