From 1778afb8f7922317cd34bbd2da13d8481866ef0f Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 21 Apr 2026 16:33:55 -0700 Subject: [PATCH] fix(track-changes): split independent replacement sidebar updates --- .../v1/extensions/comment/comments-plugin.js | 103 +++++++++++++++--- .../comment/comments-plugin.test.js | 95 ++++++++++++++++ ...ked-change-independent-replacement.spec.ts | 34 ++++++ 3 files changed, 217 insertions(+), 15 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 72c70cd49e..cdb7b6dfdf 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 @@ -877,22 +877,49 @@ const handleTrackedChangeTransaction = (trackedChangeMeta, trackedChanges, newEd } const newTrackedChanges = { ...trackedChanges }; - let id = insertedMark?.attrs?.id || deletionMark?.attrs?.id || formatMark?.attrs?.id; + const insertedId = insertedMark?.attrs?.id ?? null; + const deletionId = deletionMark?.attrs?.id ?? null; + const formatId = formatMark?.attrs?.id ?? null; + const primaryId = insertedId || deletionId || formatId; - if (!id) { + if (!primaryId) { return trackedChanges; } - // Maintain a map of tracked changes with their inserted/deleted ids - let isNewChange = false; - if (!newTrackedChanges[id]) { - newTrackedChanges[id] = {}; - isNewChange = true; - } + const registerTrackedChangeId = (changeId, patch) => { + if (!changeId) return false; + + const existing = newTrackedChanges[changeId]; + if (existing) { + Object.assign(existing, patch); + return false; + } + + newTrackedChanges[changeId] = { ...patch }; + return true; + }; + + const buildTrackedChangePayload = ({ event, marks, nodes, deletionNodes = [] }) => { + if (!marks.insertedMark && !marks.deletionMark && !marks.formatMark) { + return null; + } + + const trackedMarkId = + marks.insertedMark?.attrs?.id ?? marks.deletionMark?.attrs?.id ?? marks.formatMark?.attrs?.id ?? null; + if (!trackedMarkId) { + return null; + } - if (insertedMark) newTrackedChanges[id].insertion = id; - if (deletionMark) newTrackedChanges[id].deletion = deletionMark.attrs?.id; - if (formatMark) newTrackedChanges[id].format = formatMark.attrs?.id; + return createOrUpdateTrackedChangeComment({ + documentId: editor.options.documentId, + event, + marks, + deletionNodes, + nodes, + newEditorState, + trackedChangesForId: getTrackChanges(newEditorState, trackedMarkId), + }); + }; const { step } = trackedChangeMeta; let nodes = step?.slice?.content?.content || []; @@ -909,9 +936,54 @@ const handleTrackedChangeTransaction = (trackedChangeMeta, trackedChanges, newEd } const hasCandidateNodes = nodes.length > 0 || Boolean(deletionNodes?.length); + const hasIndependentReplacementIds = + Boolean(insertedMark && deletionMark) && Boolean(insertedId) && Boolean(deletionId) && insertedId !== deletionId; + + if (hasIndependentReplacementIds) { + const isNewInsertion = registerTrackedChangeId(insertedId, { insertion: insertedId }); + const isNewDeletion = registerTrackedChangeId(deletionId, { deletion: deletionId }); + + const insertionPayload = hasCandidateNodes + ? buildTrackedChangePayload({ + event: isNewInsertion ? 'add' : 'update', + marks: { + insertedMark, + deletionMark: null, + formatMark: null, + }, + deletionNodes: [], + nodes, + }) + : null; + + const deletionPayload = + deletionMark && (hasCandidateNodes || getTrackChanges(newEditorState, deletionId).length > 0) + ? buildTrackedChangePayload({ + event: isNewDeletion ? 'add' : 'update', + marks: { + insertedMark: null, + deletionMark, + formatMark: null, + }, + deletionNodes, + nodes: [], + }) + : null; + + if (emitCommentEvent && insertionPayload) editor.emit('commentsUpdate', insertionPayload); + if (emitCommentEvent && deletionPayload) editor.emit('commentsUpdate', deletionPayload); + return newTrackedChanges; + } + + // Maintain a map of tracked changes with their inserted/deleted ids. + const isNewChange = registerTrackedChangeId(primaryId, { + ...(insertedMark ? { insertion: primaryId } : {}), + ...(deletionMark ? { deletion: deletionId } : {}), + ...(formatMark ? { format: formatId } : {}), + }); + const emitParams = hasCandidateNodes - ? createOrUpdateTrackedChangeComment({ - documentId: editor.options.documentId, + ? buildTrackedChangePayload({ event: isNewChange ? 'add' : 'update', marks: { insertedMark, @@ -920,7 +992,6 @@ const handleTrackedChangeTransaction = (trackedChangeMeta, trackedChanges, newEd }, deletionNodes, nodes, - newEditorState, }) : null; @@ -1064,7 +1135,9 @@ const createOrUpdateTrackedChangeComment = ({ const { author, authorEmail, authorImage, date, importedAuthor } = attrs; const id = attrs.id; - let isReplacement = !!(marks.insertedMark && marks.deletionMark); + const insertedMarkId = marks.insertedMark?.attrs?.id ?? null; + const deletionMarkId = marks.deletionMark?.attrs?.id ?? null; + let isReplacement = Boolean(insertedMarkId && deletionMarkId && insertedMarkId === deletionMarkId); // Fallback: check the document for both mark types under the same ID // (covers edge cases where transaction meta only carries one mark) 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 79b36f7d0e..7d300ae1b9 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 @@ -994,6 +994,65 @@ describe('internal helper functions', () => { ); }); + it('handleTrackedChangeTransaction emits separate events for independent replacements', () => { + const schema = createCommentSchema(); + const insertMark = schema.marks[TrackInsertMarkName].create({ + id: 'change-insert-only', + author: 'Alice', + authorEmail: 'alice@example.com', + date: 'today', + }); + const deleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'change-delete-only', + author: 'Alice', + authorEmail: 'alice@example.com', + date: 'today', + }); + + const deletedNode = schema.text('Removed', [deleteMark]); + const insertedNode = schema.text('Added', [insertMark]); + const paragraph = schema.node('paragraph', null, [deletedNode, insertedNode]); + const doc = schema.node('doc', null, [paragraph]); + const state = EditorState.create({ schema, doc }); + const editor = { options: { documentId: 'doc-1' }, emit: vi.fn() }; + + const meta = { + insertedMark: insertMark, + deletionMark: deleteMark, + formatMark: null, + deletionNodes: [deletedNode], + step: { slice: { content: { content: [insertedNode] } } }, + }; + + const trackedChanges = handleTrackedChangeTransaction(meta, {}, state, editor); + + expect(trackedChanges['change-insert-only']).toMatchObject({ insertion: 'change-insert-only' }); + expect(trackedChanges['change-delete-only']).toMatchObject({ deletion: 'change-delete-only' }); + expect(editor.emit).toHaveBeenCalledTimes(2); + expect(editor.emit).toHaveBeenNthCalledWith( + 1, + 'commentsUpdate', + expect.objectContaining({ + event: comments_module_events.ADD, + changeId: 'change-insert-only', + trackedChangeType: TrackInsertMarkName, + trackedChangeText: 'Added', + deletedText: null, + }), + ); + expect(editor.emit).toHaveBeenNthCalledWith( + 2, + 'commentsUpdate', + expect.objectContaining({ + event: comments_module_events.ADD, + changeId: 'change-delete-only', + trackedChangeType: TrackDeleteMarkName, + trackedChangeText: '', + deletedText: 'Removed', + }), + ); + }); + it('handleTrackedChangeTransaction returns original state when no marks provided', () => { const schema = createCommentSchema(); const doc = schema.node('doc', null, [schema.node('paragraph', null, [schema.text('Text')])]); @@ -1167,6 +1226,42 @@ describe('internal helper functions', () => { expect(payload?.trackedChangeDisplayType).toBeNull(); }); + it('does not collapse distinct insert/delete ids into one replacement payload', () => { + const schema = createCommentSchema(); + const insertMark = schema.marks[TrackInsertMarkName].create({ + id: 'replace-insert-1', + author: 'Author', + authorEmail: 'author@example.com', + date: 'today', + }); + const deleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'replace-delete-1', + author: 'Author', + authorEmail: 'author@example.com', + date: 'today', + }); + + const docInsertNode = schema.text('replacement', [insertMark]); + const docDeleteNode = schema.text('original', [deleteMark]); + const doc = schema.node('doc', null, [schema.node('paragraph', null, [docDeleteNode, docInsertNode])]); + const state = EditorState.create({ schema, doc }); + + const payload = createOrUpdateTrackedChangeComment({ + event: 'add', + marks: { insertedMark: insertMark, deletionMark: deleteMark, formatMark: null }, + deletionNodes: [docDeleteNode], + nodes: [docInsertNode], + newEditorState: state, + documentId: 'doc-1', + trackedChangesForId: [{ mark: insertMark, from: 1, to: doc.content.size }], + }); + + expect(payload?.changeId).toBe('replace-insert-1'); + expect(payload?.trackedChangeType).toBe(TrackInsertMarkName); + expect(payload?.trackedChangeText).toBe('replacement'); + expect(payload?.deletedText).toBe(''); + }); + it('createOrUpdateTrackedChangeComment builds add and update payloads', () => { const schema = createCommentSchema(); const insertMark = schema.marks[TrackInsertMarkName].create({ diff --git a/tests/behavior/tests/comments/tracked-change-independent-replacement.spec.ts b/tests/behavior/tests/comments/tracked-change-independent-replacement.spec.ts index 6d774d3c5e..729eb48154 100644 --- a/tests/behavior/tests/comments/tracked-change-independent-replacement.spec.ts +++ b/tests/behavior/tests/comments/tracked-change-independent-replacement.spec.ts @@ -80,6 +80,40 @@ test.describe("trackedChanges.replacements='independent'", () => { expect(uniqueIds.size).toBe(allIds.length); }); + test('body replacement sidebar shows separate added and deleted bubbles', async ({ superdoc }) => { + await assertDocumentApiReady(superdoc.page); + + await superdoc.type('Replace ME now'); + await superdoc.waitForStable(); + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + await superdoc.tripleClickLine(0); + await superdoc.waitForStable(); + await superdoc.type('Replace it now'); + await superdoc.waitForStable(); + + await expect.poll(async () => (await listTrackChanges(superdoc.page)).total).toBeGreaterThanOrEqual(2); + + const dialogs = superdoc.page.locator('.comment-placeholder .comments-dialog', { + has: superdoc.page.locator('.tracked-change-text'), + }); + await expect(dialogs).toHaveCount(2); + await expect( + superdoc.page.locator('.comment-placeholder .comments-dialog .change-type', { hasText: 'Replaced' }), + ).toHaveCount(0); + + const deletedDialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { + has: superdoc.page.locator('.tracked-change-text.is-deleted', { hasText: 'ME' }), + }); + const insertedDialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { + has: superdoc.page.locator('.tracked-change-text.is-inserted', { hasText: 'it' }), + }); + + await expect(deletedDialog).toHaveCount(1); + await expect(insertedDialog).toHaveCount(1); + }); + test('accepting the insertion leaves the deletion addressable on its own', async ({ superdoc }) => { await assertDocumentApiReady(superdoc.page);