From 79ed580e6d4457b7201101ef8e287d628ee0cb4c Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Wed, 29 Apr 2026 11:50:39 +0530 Subject: [PATCH 1/6] 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 --- .../v1/extensions/comment/comments-plugin.js | 37 ++++- .../comment/comments-plugin.test.js | 149 ++++++++++++++++++ 2 files changed, 183 insertions(+), 3 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index cdb7b6dfdf..e13af5aa46 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js @@ -449,6 +449,11 @@ export const CommentsPlugin = Extension.create({ const highlightColors = editor.options.comments?.highlightColors || {}; return { activeThreadId: null, + // When a comment is activated via explicit user action (e.g., sidebar click), + // this flag prevents position-based detection from overriding it on subsequent + // DOM-sync selection transactions. Cleared when the cursor moves outside the + // comment range, allowing natural deactivation on user navigation. + explicitlySetThreadId: null, externalColor: highlightColors.external ?? '#B1124B', internalColor: highlightColors.internal ?? '#078383', decorations: DecorationSet.empty, @@ -483,6 +488,7 @@ export const CommentsPlugin = Extension.create({ return { ...pluginState, activeThreadId: newActiveThreadId, + explicitlySetThreadId: newActiveThreadId, }; } @@ -507,11 +513,30 @@ 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; + + // If the active comment was explicitly set (e.g., sidebar click) and the + // cursor is still within that comment's range, preserve it. This prevents + // DOM-sync selection transactions from overriding the explicit activation + // via position-based detection — which can fail inside tracked changes + // or structured content, causing a flicker loop. + if ( + pluginState.explicitlySetThreadId && + pluginState.explicitlySetThreadId === pluginState.activeThreadId && + selectionContainsThread(newEditorState.doc, selection, pluginState.explicitlySetThreadId) + ) { + // Selection is still inside the explicitly activated comment — skip detection. + return { ...pluginState }; + } + + // Cursor moved outside the explicitly set comment — clear the flag + // and allow position-based detection to take over. + if (pluginState.explicitlySetThreadId) { + pluginState.explicitlySetThreadId = null; + } + let currentActiveThread = getActiveCommentId(newEditorState.doc, selection); - if (trChangedActiveComment) currentActiveThread = meta.activeThreadId; if ( meta?.type === 'setCursorById' && meta.preferredActiveThreadId && @@ -534,6 +559,12 @@ export const CommentsPlugin = Extension.create({ } } + // Clear explicit flag on document changes — the user is editing, + // so position-based detection should resume. + if (tr.docChanged && pluginState.explicitlySetThreadId) { + pluginState.explicitlySetThreadId = null; + } + return { ...pluginState }; }, }, diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js index 7d300ae1b9..9cdd0712e6 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js @@ -742,6 +742,155 @@ describe('CommentsPlugin state', () => { const pluginState = CommentsPluginKey.getState(view.state); expect(pluginState.activeThreadId).toBe('tracked-1'); }); + + it('sets explicitlySetThreadId when setActiveComment meta is dispatched', () => { + const { view } = createPluginStateEnvironment(); + + const tr = view.state.tr.setMeta(CommentsPluginKey, { + type: 'setActiveComment', + activeThreadId: 'thread-1', + forceUpdate: true, + }); + + view.dispatch(tr); + + const pluginState = CommentsPluginKey.getState(view.state); + expect(pluginState.activeThreadId).toBe('thread-1'); + expect(pluginState.explicitlySetThreadId).toBe('thread-1'); + }); + + it('preserves explicitly set comment on selection-only transaction when cursor stays inside comment', () => { + const schema = createCommentSchema(); + const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true }); + // "Hello" at positions 1-6, all covered by comment mark + const paragraph = schema.node('paragraph', null, [schema.text('Hello', [commentMark])]); + const doc = schema.node('doc', null, [paragraph]); + const { view, editor } = createPluginStateEnvironment({ schema, doc }); + + // Step 1: Explicitly activate the comment + const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { + type: 'setActiveComment', + activeThreadId: 'thread-1', + forceUpdate: true, + }); + view.dispatch(setActiveTr); + expect(CommentsPluginKey.getState(view.state).activeThreadId).toBe('thread-1'); + + // Step 2: Simulate a DOM-sync selection-only transaction within the same comment range + const selectionTr = view.state.tr.setSelection(TextSelection.create(view.state.doc, 3)); + view.dispatch(selectionTr); + + // Active comment should be preserved — no commentsUpdate emitted + const pluginState = CommentsPluginKey.getState(view.state); + expect(pluginState.activeThreadId).toBe('thread-1'); + expect(pluginState.explicitlySetThreadId).toBe('thread-1'); + }); + + it('clears explicitlySetThreadId when cursor moves outside comment range', () => { + const schema = createCommentSchema(); + const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true }); + // "Hello World" — only "Hello" (pos 1-6) has the comment, " World" (pos 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 } = createPluginStateEnvironment({ schema, doc }); + + // Step 1: Explicitly activate the comment + const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { + type: 'setActiveComment', + activeThreadId: 'thread-1', + forceUpdate: true, + }); + view.dispatch(setActiveTr); + expect(CommentsPluginKey.getState(view.state).explicitlySetThreadId).toBe('thread-1'); + + // Step 2: Move cursor outside the comment range + const selectionTr = view.state.tr.setSelection(TextSelection.create(view.state.doc, 8)); + view.dispatch(selectionTr); + + // Flag should be cleared, position-based detection takes over + const pluginState = CommentsPluginKey.getState(view.state); + expect(pluginState.explicitlySetThreadId).toBeNull(); + }); + + it('preserves explicitly set comment when text has both comment and tracked-change marks', () => { + const schema = createCommentSchema(); + const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true }); + const trackedMark = schema.marks[TrackInsertMarkName].create({ id: 'tc-1' }); + // Text has both comment and tracked-change marks — the triggering scenario for the flicker bug + const paragraph = schema.node('paragraph', null, [schema.text('Hello', [commentMark, trackedMark])]); + const doc = schema.node('doc', null, [paragraph]); + const { view, editor } = createPluginStateEnvironment({ schema, doc }); + + // Step 1: Explicitly activate the comment (simulates sidebar click) + const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { + type: 'setActiveComment', + activeThreadId: 'thread-1', + forceUpdate: true, + }); + view.dispatch(setActiveTr); + expect(CommentsPluginKey.getState(view.state).activeThreadId).toBe('thread-1'); + + // Step 2: Simulate DOM-sync selection transaction (the one that caused flicker) + editor.emit.mockClear(); + const selectionTr = view.state.tr.setSelection(TextSelection.create(view.state.doc, 2)); + view.dispatch(selectionTr); + + // Active comment should be preserved — NOT overridden by position-based detection + const pluginState = CommentsPluginKey.getState(view.state); + expect(pluginState.activeThreadId).toBe('thread-1'); + expect(pluginState.explicitlySetThreadId).toBe('thread-1'); + }); + + it('allows position-based detection for range selections even when flag is set', () => { + 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 { view } = createPluginStateEnvironment({ schema, doc }); + + // Step 1: Explicitly activate the comment + const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { + type: 'setActiveComment', + activeThreadId: 'thread-1', + forceUpdate: true, + }); + view.dispatch(setActiveTr); + + // Step 2: Create a range selection (non-collapsed) within the comment + // selectionContainsThread returns false for range selections ($from !== $to), + // so the guard should NOT fire and position-based detection should run + const rangeTr = view.state.tr.setSelection(TextSelection.create(view.state.doc, 1, 4)); + view.dispatch(rangeTr); + + // The flag should be cleared since selectionContainsThread returns false for ranges + const pluginState = CommentsPluginKey.getState(view.state); + expect(pluginState.explicitlySetThreadId).toBeNull(); + }); + + it('clears explicitlySetThreadId on document change', () => { + 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 { view } = createPluginStateEnvironment({ schema, doc }); + + // Step 1: Explicitly activate the comment + const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { + type: 'setActiveComment', + activeThreadId: 'thread-1', + forceUpdate: true, + }); + view.dispatch(setActiveTr); + expect(CommentsPluginKey.getState(view.state).explicitlySetThreadId).toBe('thread-1'); + + // Step 2: Edit the document (insert text) + const editTr = view.state.tr.insertText('!', 3); + view.dispatch(editTr); + + // Flag should be cleared — user is editing, position-based detection resumes + const pluginState = CommentsPluginKey.getState(view.state); + expect(pluginState.explicitlySetThreadId).toBeNull(); + }); }); describe('normalizeCommentEventPayload', () => { From c9030c5d9444a76cbee655b775f9820417f3b543 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 29 Apr 2026 07:14:53 -0300 Subject: [PATCH 2/6] 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. --- .../v1/extensions/comment/comments-plugin.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index e13af5aa46..e54264a0dd 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js @@ -546,7 +546,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 = { From f0c7b571ff7fb7b534d067ee083faac768d5040a Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 29 Apr 2026 07:15:06 -0300 Subject: [PATCH 3/6] 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. --- .../comment/comments-plugin.test.js | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js index 9cdd0712e6..85dc17713f 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js @@ -1790,6 +1790,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 = { From 8d6c7699cb9d98da770334fc024f8f257f7b2ad8 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 29 Apr 2026 07:15:14 -0300 Subject: [PATCH 4/6] 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. --- .../comment-on-tracked-change.spec.ts | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/tests/behavior/tests/comments/comment-on-tracked-change.spec.ts b/tests/behavior/tests/comments/comment-on-tracked-change.spec.ts index d7228e2308..873e14f7cd 100644 --- a/tests/behavior/tests/comments/comment-on-tracked-change.spec.ts +++ b/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); +}); From f2c3d05bb2bc9067cb841e13fad5b463fc7a5337 Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Wed, 29 Apr 2026 15:59:57 +0530 Subject: [PATCH 5/6] fix(comments): write explicitlySetThreadId on copy, not original state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback — avoid mutating pluginState directly before spreading. Use conditional copy pattern consistent with Caio's suggestion. --- .../v1/extensions/comment/comments-plugin.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index e54264a0dd..3b62ff3417 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js @@ -532,9 +532,9 @@ export const CommentsPlugin = Extension.create({ // Cursor moved outside the explicitly set comment — clear the flag // and allow position-based detection to take over. - if (pluginState.explicitlySetThreadId) { - pluginState.explicitlySetThreadId = null; - } + pluginState = pluginState.explicitlySetThreadId + ? { ...pluginState, explicitlySetThreadId: null } + : pluginState; let currentActiveThread = getActiveCommentId(newEditorState.doc, selection); if ( @@ -574,9 +574,10 @@ export const CommentsPlugin = Extension.create({ // Clear explicit flag on document changes — the user is editing, // so position-based detection should resume. - if (tr.docChanged && pluginState.explicitlySetThreadId) { - pluginState.explicitlySetThreadId = null; - } + pluginState = + tr.docChanged && pluginState.explicitlySetThreadId + ? { ...pluginState, explicitlySetThreadId: null } + : pluginState; return { ...pluginState }; }, From d6463a20b3941ff42047ab7e8e3b73ef3dd2da32 Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Wed, 29 Apr 2026 16:07:27 +0530 Subject: [PATCH 6/6] 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. --- .../v1/extensions/comment/comments-plugin.js | 33 ---- .../comment/comments-plugin.test.js | 149 ------------------ 2 files changed, 182 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index 3b62ff3417..f42b4feadd 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js @@ -449,11 +449,6 @@ export const CommentsPlugin = Extension.create({ const highlightColors = editor.options.comments?.highlightColors || {}; return { activeThreadId: null, - // When a comment is activated via explicit user action (e.g., sidebar click), - // this flag prevents position-based detection from overriding it on subsequent - // DOM-sync selection transactions. Cleared when the cursor moves outside the - // comment range, allowing natural deactivation on user navigation. - explicitlySetThreadId: null, externalColor: highlightColors.external ?? '#B1124B', internalColor: highlightColors.internal ?? '#078383', decorations: DecorationSet.empty, @@ -488,7 +483,6 @@ export const CommentsPlugin = Extension.create({ return { ...pluginState, activeThreadId: newActiveThreadId, - explicitlySetThreadId: newActiveThreadId, }; } @@ -516,26 +510,6 @@ export const CommentsPlugin = Extension.create({ if (!tr.docChanged && tr.selectionSet) { const { selection } = tr; - // If the active comment was explicitly set (e.g., sidebar click) and the - // cursor is still within that comment's range, preserve it. This prevents - // DOM-sync selection transactions from overriding the explicit activation - // via position-based detection — which can fail inside tracked changes - // or structured content, causing a flicker loop. - if ( - pluginState.explicitlySetThreadId && - pluginState.explicitlySetThreadId === pluginState.activeThreadId && - selectionContainsThread(newEditorState.doc, selection, pluginState.explicitlySetThreadId) - ) { - // Selection is still inside the explicitly activated comment — skip detection. - return { ...pluginState }; - } - - // Cursor moved outside the explicitly set comment — clear the flag - // and allow position-based detection to take over. - pluginState = pluginState.explicitlySetThreadId - ? { ...pluginState, explicitlySetThreadId: null } - : pluginState; - let currentActiveThread = getActiveCommentId(newEditorState.doc, selection); if ( meta?.type === 'setCursorById' && @@ -572,13 +546,6 @@ export const CommentsPlugin = Extension.create({ } } - // Clear explicit flag on document changes — the user is editing, - // so position-based detection should resume. - pluginState = - tr.docChanged && pluginState.explicitlySetThreadId - ? { ...pluginState, explicitlySetThreadId: null } - : pluginState; - return { ...pluginState }; }, }, diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js index 85dc17713f..6236ae3f07 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js @@ -742,155 +742,6 @@ describe('CommentsPlugin state', () => { const pluginState = CommentsPluginKey.getState(view.state); expect(pluginState.activeThreadId).toBe('tracked-1'); }); - - it('sets explicitlySetThreadId when setActiveComment meta is dispatched', () => { - const { view } = createPluginStateEnvironment(); - - const tr = view.state.tr.setMeta(CommentsPluginKey, { - type: 'setActiveComment', - activeThreadId: 'thread-1', - forceUpdate: true, - }); - - view.dispatch(tr); - - const pluginState = CommentsPluginKey.getState(view.state); - expect(pluginState.activeThreadId).toBe('thread-1'); - expect(pluginState.explicitlySetThreadId).toBe('thread-1'); - }); - - it('preserves explicitly set comment on selection-only transaction when cursor stays inside comment', () => { - const schema = createCommentSchema(); - const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true }); - // "Hello" at positions 1-6, all covered by comment mark - const paragraph = schema.node('paragraph', null, [schema.text('Hello', [commentMark])]); - const doc = schema.node('doc', null, [paragraph]); - const { view, editor } = createPluginStateEnvironment({ schema, doc }); - - // Step 1: Explicitly activate the comment - const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { - type: 'setActiveComment', - activeThreadId: 'thread-1', - forceUpdate: true, - }); - view.dispatch(setActiveTr); - expect(CommentsPluginKey.getState(view.state).activeThreadId).toBe('thread-1'); - - // Step 2: Simulate a DOM-sync selection-only transaction within the same comment range - const selectionTr = view.state.tr.setSelection(TextSelection.create(view.state.doc, 3)); - view.dispatch(selectionTr); - - // Active comment should be preserved — no commentsUpdate emitted - const pluginState = CommentsPluginKey.getState(view.state); - expect(pluginState.activeThreadId).toBe('thread-1'); - expect(pluginState.explicitlySetThreadId).toBe('thread-1'); - }); - - it('clears explicitlySetThreadId when cursor moves outside comment range', () => { - const schema = createCommentSchema(); - const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true }); - // "Hello World" — only "Hello" (pos 1-6) has the comment, " World" (pos 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 } = createPluginStateEnvironment({ schema, doc }); - - // Step 1: Explicitly activate the comment - const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { - type: 'setActiveComment', - activeThreadId: 'thread-1', - forceUpdate: true, - }); - view.dispatch(setActiveTr); - expect(CommentsPluginKey.getState(view.state).explicitlySetThreadId).toBe('thread-1'); - - // Step 2: Move cursor outside the comment range - const selectionTr = view.state.tr.setSelection(TextSelection.create(view.state.doc, 8)); - view.dispatch(selectionTr); - - // Flag should be cleared, position-based detection takes over - const pluginState = CommentsPluginKey.getState(view.state); - expect(pluginState.explicitlySetThreadId).toBeNull(); - }); - - it('preserves explicitly set comment when text has both comment and tracked-change marks', () => { - const schema = createCommentSchema(); - const commentMark = schema.marks[CommentMarkName].create({ commentId: 'thread-1', internal: true }); - const trackedMark = schema.marks[TrackInsertMarkName].create({ id: 'tc-1' }); - // Text has both comment and tracked-change marks — the triggering scenario for the flicker bug - const paragraph = schema.node('paragraph', null, [schema.text('Hello', [commentMark, trackedMark])]); - const doc = schema.node('doc', null, [paragraph]); - const { view, editor } = createPluginStateEnvironment({ schema, doc }); - - // Step 1: Explicitly activate the comment (simulates sidebar click) - const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { - type: 'setActiveComment', - activeThreadId: 'thread-1', - forceUpdate: true, - }); - view.dispatch(setActiveTr); - expect(CommentsPluginKey.getState(view.state).activeThreadId).toBe('thread-1'); - - // Step 2: Simulate DOM-sync selection transaction (the one that caused flicker) - editor.emit.mockClear(); - const selectionTr = view.state.tr.setSelection(TextSelection.create(view.state.doc, 2)); - view.dispatch(selectionTr); - - // Active comment should be preserved — NOT overridden by position-based detection - const pluginState = CommentsPluginKey.getState(view.state); - expect(pluginState.activeThreadId).toBe('thread-1'); - expect(pluginState.explicitlySetThreadId).toBe('thread-1'); - }); - - it('allows position-based detection for range selections even when flag is set', () => { - 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 { view } = createPluginStateEnvironment({ schema, doc }); - - // Step 1: Explicitly activate the comment - const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { - type: 'setActiveComment', - activeThreadId: 'thread-1', - forceUpdate: true, - }); - view.dispatch(setActiveTr); - - // Step 2: Create a range selection (non-collapsed) within the comment - // selectionContainsThread returns false for range selections ($from !== $to), - // so the guard should NOT fire and position-based detection should run - const rangeTr = view.state.tr.setSelection(TextSelection.create(view.state.doc, 1, 4)); - view.dispatch(rangeTr); - - // The flag should be cleared since selectionContainsThread returns false for ranges - const pluginState = CommentsPluginKey.getState(view.state); - expect(pluginState.explicitlySetThreadId).toBeNull(); - }); - - it('clears explicitlySetThreadId on document change', () => { - 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 { view } = createPluginStateEnvironment({ schema, doc }); - - // Step 1: Explicitly activate the comment - const setActiveTr = view.state.tr.setMeta(CommentsPluginKey, { - type: 'setActiveComment', - activeThreadId: 'thread-1', - forceUpdate: true, - }); - view.dispatch(setActiveTr); - expect(CommentsPluginKey.getState(view.state).explicitlySetThreadId).toBe('thread-1'); - - // Step 2: Edit the document (insert text) - const editTr = view.state.tr.insertText('!', 3); - view.dispatch(editTr); - - // Flag should be cleared — user is editing, position-based detection resumes - const pluginState = CommentsPluginKey.getState(view.state); - expect(pluginState.explicitlySetThreadId).toBeNull(); - }); }); describe('normalizeCommentEventPayload', () => {