From 127a354f248e151cc3ec2ed2828c43c0a81825c7 Mon Sep 17 00:00:00 2001 From: Clarence Palmer Date: Mon, 23 Feb 2026 21:59:41 -0800 Subject: [PATCH 1/3] feat(track-changes): enhance rejection and acceptance of tracked changes in toolbar --- .../track-changes-extension.test.js | 131 ++++++++++++++++++ .../extensions/track-changes/track-changes.js | 25 ++++ .../CommentsLayer/FloatingComments.vue | 22 +++ .../superdoc/src/stores/comments-store.js | 17 ++- .../src/stores/comments-store.test.js | 76 ++++++++++ ...reject-format-suggestion-selection.spec.ts | 62 +++++++++ .../reject-mixed-track-format.spec.ts | 60 ++++++++ 7 files changed, 391 insertions(+), 2 deletions(-) create mode 100644 tests/behavior/tests/comments/reject-format-suggestion-selection.spec.ts create mode 100644 tests/visual/tests/behavior/formatting/reject-mixed-track-format.spec.ts diff --git a/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js b/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js index ed48431114..394c32be03 100644 --- a/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js +++ b/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js @@ -146,6 +146,36 @@ describe('TrackChanges extension commands', () => { expect(markPresent(restoredState.doc, TrackDeleteMarkName)).toBe(false); }); + it('rejectTrackedChangesBetween emits tracked-change resolve events for rejected IDs', () => { + const insertMark = schema.marks[TrackInsertMarkName].create({ id: 'ins-resolve-1' }); + const doc = createDoc('Pending', [insertMark]); + const state = createState(doc); + const emit = vi.fn(); + + commands.rejectTrackedChangesBetween( + 1, + doc.content.size, + )({ + state, + dispatch: (tr) => state.apply(tr), + editor: { + emit, + options: { user: { email: 'reviewer@example.com', name: 'Reviewer' } }, + }, + }); + + expect(emit).toHaveBeenCalledWith( + 'commentsUpdate', + expect.objectContaining({ + type: 'trackedChange', + event: 'resolve', + changeId: 'ins-resolve-1', + resolvedByEmail: 'reviewer@example.com', + resolvedByName: 'Reviewer', + }), + ); + }); + it('blocks rejecting tracked changes when permissionResolver denies access', () => { const deleteMark = schema.marks[TrackDeleteMarkName].create({ id: 'del-guard', authorEmail: 'author@example.com' }); const doc = createDoc('Legacy', [deleteMark]); @@ -211,6 +241,59 @@ describe('TrackChanges extension commands', () => { expect(markPresent(afterReject.doc, 'italic')).toBe(false); }); + it('acceptTrackedChangesBetween bulk-accepts all format/style changes in range', () => { + const bold = schema.marks.bold.create(); + const italic = schema.marks.italic.create(); + const fmt1 = schema.marks[TrackFormatMarkName].create({ + id: 'fmt-bulk-1', + before: [], + after: [{ type: 'bold', attrs: {} }], + }); + const fmt2 = schema.marks[TrackFormatMarkName].create({ + id: 'fmt-bulk-2', + before: [{ type: 'bold', attrs: {} }], + after: [{ type: 'italic', attrs: {} }], + }); + const paragraph = schema.nodes.paragraph.create(null, [ + schema.text('One', [bold, fmt1]), + schema.text(' two ', []), + schema.text('three', [italic, fmt2]), + ]); + const doc = schema.nodes.doc.create(null, paragraph); + const state = createState(doc); + + let afterAccept; + commands.acceptTrackedChangesBetween( + 0, + doc.content.size, + )({ + state, + dispatch: (tr) => { + afterAccept = state.apply(tr); + }, + }); + + expect(afterAccept).toBeDefined(); + expect(afterAccept.doc.textContent).toBe('One two three'); + + let formatMarkCount = 0; + afterAccept.doc.descendants((node) => { + if (node.marks.some((m) => m.type.name === TrackFormatMarkName)) formatMarkCount += 1; + }); + expect(formatMarkCount).toBe(0); + + const firstRange = getFirstTextRange(afterAccept.doc); + const firstMarks = afterAccept.doc.nodeAt(firstRange.from)?.marks ?? []; + expect(firstMarks.some((m) => m.type.name === 'bold')).toBe(true); + + afterAccept.doc.descendants((node, pos) => { + if (!node.isText || node.textContent !== 'three') return; + const marks = afterAccept.doc.nodeAt(pos)?.marks ?? []; + expect(marks.some((m) => m.type.name === 'italic')).toBe(true); + return false; + }); + }); + it('rejectTrackedChangesBetween restores imported textStyle attrs for color suggestions', () => { const oldTextStyle = schema.marks.textStyle.create({ styleId: 'Emphasis', @@ -461,6 +544,54 @@ describe('TrackChanges extension commands', () => { } }); + it('interaction: rejectTrackedChangeOnSelection reverts mixed marks + textStyle in suggesting mode', () => { + const { editor: interactionEditor } = initTestEditor({ + mode: 'text', + content: '

Agreement signed by both parties

', + user: { name: 'Track Tester', email: 'track@example.com' }, + }); + + try { + const textRange = getFirstTextRange(interactionEditor.state.doc); + expect(textRange).toBeDefined(); + + interactionEditor.view.dispatch( + interactionEditor.state.tr.setSelection( + TextSelection.create(interactionEditor.state.doc, textRange.from, textRange.to), + ), + ); + interactionEditor.commands.setFontFamily('Times New Roman, serif'); + interactionEditor.commands.setColor('#112233'); + interactionEditor.setDocumentMode('suggesting'); + + const selectionRange = getFirstTextRange(interactionEditor.state.doc); + interactionEditor.view.dispatch( + interactionEditor.state.tr.setSelection( + TextSelection.create(interactionEditor.state.doc, selectionRange.from, selectionRange.to), + ), + ); + interactionEditor.commands.toggleBold(); + interactionEditor.commands.toggleUnderline(); + interactionEditor.commands.setColor('#FF00AA'); + interactionEditor.commands.setFontFamily('Arial, sans-serif'); + + interactionEditor.commands.rejectTrackedChangeOnSelection(); + + const textPos = getFirstTextRange(interactionEditor.state.doc); + const textNode = interactionEditor.state.doc.nodeAt(textPos.from); + const marks = textNode?.marks || []; + const textStyle = marks.find((mark) => mark.type.name === 'textStyle'); + + expect(marks.some((mark) => mark.type.name === TrackFormatMarkName)).toBe(false); + expect(marks.some((mark) => mark.type.name === 'bold')).toBe(false); + expect(marks.some((mark) => mark.type.name === 'underline')).toBe(false); + expect(textStyle?.attrs?.color).toBe('#112233'); + expect(textStyle?.attrs?.fontFamily).toBe('Times New Roman, serif'); + } finally { + interactionEditor.destroy(); + } + }); + it('acceptTrackedChangeById links contiguous insertion segments sharing an id across formatting', () => { const italicMark = schema.marks.italic.create(); const insertionId = 'ins-multi'; diff --git a/packages/super-editor/src/extensions/track-changes/track-changes.js b/packages/super-editor/src/extensions/track-changes/track-changes.js index c09c1beccd..6636164fbd 100644 --- a/packages/super-editor/src/extensions/track-changes/track-changes.js +++ b/packages/super-editor/src/extensions/track-changes/track-changes.js @@ -80,6 +80,9 @@ export const TrackChanges = Extension.create({ if (!isTrackedChangeActionAllowed({ editor, action: 'reject', trackedChanges })) return false; const { tr, doc } = state; + // Keep the IDs rejected in this transaction so the comments layer can + // resolve/hide the corresponding tracked-change bubbles in one path. + const rejectedChangeIds = new Set(); // tr.setMeta('acceptReject', true); tr.setMeta('inputType', 'acceptReject'); @@ -89,6 +92,7 @@ export const TrackChanges = Extension.create({ doc.nodesBetween(from, to, (node, pos) => { if (node.marks && node.marks.find((mark) => mark.type.name === TrackDeleteMarkName)) { const deletionMark = node.marks.find((mark) => mark.type.name === TrackDeleteMarkName); + if (deletionMark?.attrs?.id) rejectedChangeIds.add(deletionMark.attrs.id); tr.step( new RemoveMarkStep( @@ -98,6 +102,9 @@ export const TrackChanges = Extension.create({ ), ); } else if (node.marks && node.marks.find((mark) => mark.type.name === TrackInsertMarkName)) { + const insertionMark = node.marks.find((mark) => mark.type.name === TrackInsertMarkName); + if (insertionMark?.attrs?.id) rejectedChangeIds.add(insertionMark.attrs.id); + const deletionStep = new ReplaceStep( map.map(Math.max(pos, from)), map.map(Math.min(pos + node.nodeSize, to)), @@ -108,6 +115,7 @@ export const TrackChanges = Extension.create({ map.appendMap(deletionStep.getMap()); } else if (node.marks && node.marks.find((mark) => mark.type.name === TrackFormatMarkName)) { const formatChangeMark = node.marks.find((mark) => mark.type.name === TrackFormatMarkName); + if (formatChangeMark?.attrs?.id) rejectedChangeIds.add(formatChangeMark.attrs.id); formatChangeMark.attrs.before.forEach((oldMark) => { tr.step( @@ -148,6 +156,23 @@ export const TrackChanges = Extension.create({ if (tr.steps.length) { dispatch(tr); + + if (editor?.emit && rejectedChangeIds.size) { + const resolvedByEmail = editor.options?.user?.email; + const resolvedByName = editor.options?.user?.name; + + // Bubble reject-by-selection/reject-all through the same + // tracked-change comment resolution channel used by bubble actions. + rejectedChangeIds.forEach((changeId) => { + editor.emit('commentsUpdate', { + type: 'trackedChange', + event: 'resolve', + changeId, + resolvedByEmail, + resolvedByName, + }); + }); + } } return true; diff --git a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue index 481c3626d1..62d216c3e0 100644 --- a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue +++ b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue @@ -121,6 +121,28 @@ watchEffect(() => { nextTick(processLocations); }); +watch( + () => + getFloatingComments.value + .map((comment) => String(comment.commentId || comment.importedId)) + .sort() + .join('|'), + () => { + const activeIds = new Set( + getFloatingComments.value.map((comment) => String(comment.commentId || comment.importedId)).filter(Boolean), + ); + + // Remove rendered entries for comments that no longer exist so old tracked + // change dialogs do not linger after bulk reject clears their threads. + renderedSizes.value = renderedSizes.value.filter((item) => activeIds.has(String(item.id))); + + if (!activeIds.size) { + firstGroupRendered.value = false; + verticalOffset.value = 0; + } + }, +); + watch(activeComment, (newVal, oldVal) => { nextTick(() => { if (!activeComment.value) return (verticalOffset.value = 0); diff --git a/packages/superdoc/src/stores/comments-store.js b/packages/superdoc/src/stores/comments-store.js index d0b2f485f8..246a64435a 100644 --- a/packages/superdoc/src/stores/comments-store.js +++ b/packages/superdoc/src/stores/comments-store.js @@ -360,6 +360,17 @@ export const useCommentsStore = defineStore('comments', () => { syncCommentsToClients(superdoc, emitData); debounceEmit(changeId, emitData, superdoc); + } else if (event === 'resolve') { + const existingTrackedChange = commentsList.value.find((comment) => comment.commentId === changeId); + if (!existingTrackedChange || existingTrackedChange.resolvedTime) return; + + // Selection/toolbar reject emits tracked-change resolve events. Use the same + // resolution path as the comment dialog so one method owns state + sync + emit. + existingTrackedChange.resolveComment({ + email: params.resolvedByEmail ?? superdoc?.user?.email ?? null, + name: params.resolvedByName ?? superdoc?.user?.name ?? null, + superdoc, + }); } }; @@ -823,10 +834,12 @@ export const useCommentsStore = defineStore('comments', () => { * @returns {void} */ const handleEditorLocationsUpdate = (allCommentPositions) => { - if ((!allCommentPositions || Object.keys(allCommentPositions).length === 0) && commentsList.value.length > 0) { + if (allCommentPositions == null) { return; } - editorCommentPositions.value = allCommentPositions || {}; + // `{}` is authoritative: when marks are removed, positions can become empty + // and we must clear stale anchors instead of preserving previous ones. + editorCommentPositions.value = allCommentPositions; }; /** diff --git a/packages/superdoc/src/stores/comments-store.test.js b/packages/superdoc/src/stores/comments-store.test.js index 561319dba8..4b94c07bd2 100644 --- a/packages/superdoc/src/stores/comments-store.test.js +++ b/packages/superdoc/src/stores/comments-store.test.js @@ -214,6 +214,56 @@ describe('comments-store', () => { ); }); + it('resolves tracked change comments on resolve events', () => { + const superdoc = { + emit: vi.fn(), + user: { email: 'reviewer@example.com', name: 'Reviewer' }, + }; + + const existingComment = { + commentId: 'change-resolve-1', + trackedChange: true, + resolvedTime: null, + resolvedByEmail: null, + resolvedByName: null, + getValues: vi.fn(() => ({ commentId: 'change-resolve-1', resolvedTime: Date.now() })), + resolveComment: vi.fn(function ({ email, name }) { + this.resolvedTime = Date.now(); + this.resolvedByEmail = email; + this.resolvedByName = name; + const emitData = { type: comments_module_events.RESOLVED, comment: this.getValues() }; + syncCommentsToClientsMock(superdoc, emitData); + superdoc.emit('comments-update', emitData); + }), + }; + store.commentsList = [existingComment]; + + store.handleTrackedChangeUpdate({ + superdoc, + params: { + event: 'resolve', + changeId: 'change-resolve-1', + }, + }); + + expect(existingComment.resolveComment).toHaveBeenCalledWith({ + email: 'reviewer@example.com', + name: 'Reviewer', + superdoc, + }); + expect(existingComment.resolvedTime).not.toBeNull(); + expect(existingComment.resolvedByEmail).toBe('reviewer@example.com'); + expect(existingComment.resolvedByName).toBe('Reviewer'); + expect(syncCommentsToClientsMock).toHaveBeenCalledWith( + superdoc, + expect.objectContaining({ type: comments_module_events.RESOLVED }), + ); + expect(superdoc.emit).toHaveBeenCalledWith( + 'comments-update', + expect.objectContaining({ type: comments_module_events.RESOLVED }), + ); + }); + it('should load comments with correct created time', () => { store.init({ readOnly: true, @@ -360,6 +410,32 @@ describe('comments-store', () => { }); }); + describe('handleEditorLocationsUpdate', () => { + it('clears stale positions when editor emits an empty positions payload', () => { + store.commentsList = [{ commentId: 'tc-1', trackedChange: true }]; + store.editorCommentPositions = { + 'tc-1': { from: 1, to: 5 }, + }; + + store.handleEditorLocationsUpdate({}); + + expect(store.editorCommentPositions).toEqual({}); + }); + + it('ignores nullish payloads to avoid clobbering valid positions', () => { + store.editorCommentPositions = { + 'tc-1': { from: 1, to: 5 }, + }; + + store.handleEditorLocationsUpdate(undefined); + store.handleEditorLocationsUpdate(null); + + expect(store.editorCommentPositions).toEqual({ + 'tc-1': { from: 1, to: 5 }, + }); + }); + }); + describe('viewing visibility filters', () => { it('hides tracked change threads when viewing mode hides tracked changes', () => { store.commentsList = [ diff --git a/tests/behavior/tests/comments/reject-format-suggestion-selection.spec.ts b/tests/behavior/tests/comments/reject-format-suggestion-selection.spec.ts new file mode 100644 index 0000000000..f3ed80ddea --- /dev/null +++ b/tests/behavior/tests/comments/reject-format-suggestion-selection.spec.ts @@ -0,0 +1,62 @@ +import type { Page } from '@playwright/test'; +import { test, expect } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'full', comments: 'panel', trackChanges: true } }); + +const TEXT = 'Agreement signed by both parties'; + +type EditorCommand = [name: string, ...args: unknown[]]; + +async function runCommands(page: Page, commands: EditorCommand[]): Promise { + for (const [name, ...args] of commands) { + await page.evaluate( + ({ commandName, commandArgs }) => (window as any).editor.commands[commandName](...commandArgs), + { + commandName: name, + commandArgs: args, + }, + ); + } +} + +test('reject tracked mixed marks + textStyle on selection restores original formatting', async ({ superdoc }) => { + await superdoc.type(TEXT); + await superdoc.waitForStable(); + + await superdoc.selectAll(); + await superdoc.waitForStable(); + await runCommands(superdoc.page, [ + ['setFontFamily', 'Times New Roman, serif'], + ['setColor', '#112233'], + ]); + await superdoc.waitForStable(); + + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + await superdoc.selectAll(); + await superdoc.waitForStable(); + await runCommands(superdoc.page, [ + ['toggleBold'], + ['toggleUnderline'], + ['setColor', '#FF00AA'], + ['setFontFamily', 'Arial, sans-serif'], + ]); + await superdoc.waitForStable(); + + await superdoc.assertTrackedChangeExists('format'); + const trackedDialog = superdoc.page.locator('.floating-comment > .comments-dialog', { + has: superdoc.page.locator('.tracked-change-text'), + }); + await expect(trackedDialog).toHaveCount(1); + + await superdoc.executeCommand('rejectTrackedChangeFromToolbar'); + await superdoc.waitForStable(); + + await expect(superdoc.page.locator('.track-format-dec')).toHaveCount(0); + await expect(trackedDialog).toHaveCount(0); + await superdoc.assertTextLacksMarks('Agreement', ['bold', 'underline']); + await superdoc.assertTextMarkAttrs('Agreement', 'textStyle', { color: '#112233' }); + await superdoc.assertTextMarkAttrs('Agreement', 'textStyle', { fontFamily: 'Times New Roman' }); + await superdoc.assertTextContent(TEXT); +}); diff --git a/tests/visual/tests/behavior/formatting/reject-mixed-track-format.spec.ts b/tests/visual/tests/behavior/formatting/reject-mixed-track-format.spec.ts new file mode 100644 index 0000000000..37868bcf82 --- /dev/null +++ b/tests/visual/tests/behavior/formatting/reject-mixed-track-format.spec.ts @@ -0,0 +1,60 @@ +import type { Page } from '@playwright/test'; +import { test, expect } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'full', comments: 'panel', trackChanges: true } }); + +const TEXT = 'Agreement signed by both parties'; + +type EditorCommand = [name: string, ...args: unknown[]]; + +async function runCommands(page: Page, commands: EditorCommand[]): Promise { + for (const [name, ...args] of commands) { + await page.evaluate( + ({ commandName, commandArgs }) => (window as any).editor.commands[commandName](...commandArgs), + { + commandName: name, + commandArgs: args, + }, + ); + } +} + +test('@behavior reject all tracked mixed formatting restores original visual state', async ({ superdoc }) => { + await superdoc.type(TEXT); + await superdoc.waitForStable(); + + await superdoc.selectAll(); + await superdoc.waitForStable(); + await runCommands(superdoc.page, [ + ['setFontFamily', 'Times New Roman, serif'], + ['setColor', '#112233'], + ]); + await superdoc.waitForStable(); + + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + await superdoc.selectAll(); + await superdoc.waitForStable(); + await runCommands(superdoc.page, [ + ['toggleBold'], + ['toggleUnderline'], + ['setColor', '#FF00AA'], + ['setFontFamily', 'Arial, sans-serif'], + ]); + await superdoc.waitForStable(); + + const trackedDialog = superdoc.page.locator('.floating-comment > .comments-dialog', { + has: superdoc.page.locator('.tracked-change-text'), + }); + await expect(trackedDialog).toHaveCount(1); + + await superdoc.screenshot('reject-mixed-track-format-before-reject'); + + await superdoc.executeCommand('rejectTrackedChangeFromToolbar'); + await superdoc.waitForStable(); + + await expect(superdoc.page.locator('.track-format-dec')).toHaveCount(0); + await expect(trackedDialog).toHaveCount(0); + await superdoc.screenshot('reject-mixed-track-format-after-reject'); +}); From 1e56dfce6a9b6bdea258f6c10062c446f24185c3 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 26 Feb 2026 14:09:49 -0800 Subject: [PATCH 2/3] fix(track-changes): expand selection reject to full tracked-change IDs --- .../track-changes-extension.test.js | 36 +++++ .../extensions/track-changes/track-changes.js | 149 +++++++++++------- 2 files changed, 129 insertions(+), 56 deletions(-) diff --git a/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js b/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js index 394c32be03..a66395308c 100644 --- a/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js +++ b/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js @@ -176,6 +176,42 @@ describe('TrackChanges extension commands', () => { ); }); + it('rejectTrackedChangesBetween expands partial selection to reject the full tracked-change ID', () => { + const changeId = 'ins-partial-still-present'; + const insertMark = schema.marks[TrackInsertMarkName].create({ id: changeId }); + const doc = createDoc('Pending', [insertMark]); + const state = createState(doc); + const emit = vi.fn(); + + let nextState; + commands.rejectTrackedChangesBetween( + 1, + 3, + )({ + state, + dispatch: (tr) => { + nextState = state.apply(tr); + }, + editor: { + emit, + options: { user: { email: 'reviewer@example.com', name: 'Reviewer' } }, + }, + }); + + expect(nextState).toBeDefined(); + expect(nextState.doc.textContent).toBe(''); + + const resolveEventsForId = emit.mock.calls.filter(([eventName, payload]) => { + return ( + eventName === 'commentsUpdate' && + payload?.type === 'trackedChange' && + payload?.event === 'resolve' && + payload?.changeId === changeId + ); + }); + expect(resolveEventsForId).toHaveLength(1); + }); + it('blocks rejecting tracked changes when permissionResolver denies access', () => { const deleteMark = schema.marks[TrackDeleteMarkName].create({ id: 'del-guard', authorEmail: 'author@example.com' }); const doc = createDoc('Legacy', [deleteMark]); diff --git a/packages/super-editor/src/extensions/track-changes/track-changes.js b/packages/super-editor/src/extensions/track-changes/track-changes.js index 6636164fbd..becdf83d24 100644 --- a/packages/super-editor/src/extensions/track-changes/track-changes.js +++ b/packages/super-editor/src/extensions/track-changes/track-changes.js @@ -76,7 +76,10 @@ export const TrackChanges = Extension.create({ rejectTrackedChangesBetween: (from, to) => ({ state, dispatch, editor }) => { - const trackedChanges = collectTrackedChanges({ state, from, to }); + const trackedChangesInSelection = collectTrackedChanges({ state, from, to }); + const trackedChangesById = getTrackedChangesByTouchedIds(state, trackedChangesInSelection); + const trackedChangesWithoutId = trackedChangesInSelection.filter(({ mark }) => !mark?.attrs?.id); + const trackedChanges = dedupeTrackedChangeRanges([...trackedChangesById, ...trackedChangesWithoutId]); if (!isTrackedChangeActionAllowed({ editor, action: 'reject', trackedChanges })) return false; const { tr, doc } = state; @@ -89,69 +92,71 @@ export const TrackChanges = Extension.create({ const map = new Mapping(); - doc.nodesBetween(from, to, (node, pos) => { - if (node.marks && node.marks.find((mark) => mark.type.name === TrackDeleteMarkName)) { - const deletionMark = node.marks.find((mark) => mark.type.name === TrackDeleteMarkName); - if (deletionMark?.attrs?.id) rejectedChangeIds.add(deletionMark.attrs.id); - - tr.step( - new RemoveMarkStep( - map.map(Math.max(pos, from)), - map.map(Math.min(pos + node.nodeSize, to)), - deletionMark, - ), - ); - } else if (node.marks && node.marks.find((mark) => mark.type.name === TrackInsertMarkName)) { - const insertionMark = node.marks.find((mark) => mark.type.name === TrackInsertMarkName); - if (insertionMark?.attrs?.id) rejectedChangeIds.add(insertionMark.attrs.id); - - const deletionStep = new ReplaceStep( - map.map(Math.max(pos, from)), - map.map(Math.min(pos + node.nodeSize, to)), - Slice.empty, - ); + trackedChanges.forEach(({ from: rangeFrom, to: rangeTo }) => { + doc.nodesBetween(rangeFrom, rangeTo, (node, pos) => { + if (node.marks && node.marks.find((mark) => mark.type.name === TrackDeleteMarkName)) { + const deletionMark = node.marks.find((mark) => mark.type.name === TrackDeleteMarkName); + if (deletionMark?.attrs?.id) rejectedChangeIds.add(deletionMark.attrs.id); - tr.step(deletionStep); - map.appendMap(deletionStep.getMap()); - } else if (node.marks && node.marks.find((mark) => mark.type.name === TrackFormatMarkName)) { - const formatChangeMark = node.marks.find((mark) => mark.type.name === TrackFormatMarkName); - if (formatChangeMark?.attrs?.id) rejectedChangeIds.add(formatChangeMark.attrs.id); - - formatChangeMark.attrs.before.forEach((oldMark) => { tr.step( - new AddMarkStep( - map.map(Math.max(pos, from)), - map.map(Math.min(pos + node.nodeSize, to)), - state.schema.marks[oldMark.type].create(oldMark.attrs), + new RemoveMarkStep( + map.map(Math.max(pos, rangeFrom)), + map.map(Math.min(pos + node.nodeSize, rangeTo)), + deletionMark, ), ); - }); + } else if (node.marks && node.marks.find((mark) => mark.type.name === TrackInsertMarkName)) { + const insertionMark = node.marks.find((mark) => mark.type.name === TrackInsertMarkName); + if (insertionMark?.attrs?.id) rejectedChangeIds.add(insertionMark.attrs.id); + + const deletionStep = new ReplaceStep( + map.map(Math.max(pos, rangeFrom)), + map.map(Math.min(pos + node.nodeSize, rangeTo)), + Slice.empty, + ); - formatChangeMark.attrs.after.forEach((newMark) => { - const mappedFrom = map.map(Math.max(pos, from)); - const mappedTo = map.map(Math.min(pos + node.nodeSize, to)); - const liveMark = findMarkInRangeBySnapshot({ - doc: tr.doc, - from: mappedFrom, - to: mappedTo, - snapshot: newMark, + tr.step(deletionStep); + map.appendMap(deletionStep.getMap()); + } else if (node.marks && node.marks.find((mark) => mark.type.name === TrackFormatMarkName)) { + const formatChangeMark = node.marks.find((mark) => mark.type.name === TrackFormatMarkName); + if (formatChangeMark?.attrs?.id) rejectedChangeIds.add(formatChangeMark.attrs.id); + + formatChangeMark.attrs.before.forEach((oldMark) => { + tr.step( + new AddMarkStep( + map.map(Math.max(pos, rangeFrom)), + map.map(Math.min(pos + node.nodeSize, rangeTo)), + state.schema.marks[oldMark.type].create(oldMark.attrs), + ), + ); }); - if (!liveMark) { - return; - } - - tr.step(new RemoveMarkStep(mappedFrom, mappedTo, liveMark)); - }); + formatChangeMark.attrs.after.forEach((newMark) => { + const mappedFrom = map.map(Math.max(pos, rangeFrom)); + const mappedTo = map.map(Math.min(pos + node.nodeSize, rangeTo)); + const liveMark = findMarkInRangeBySnapshot({ + doc: tr.doc, + from: mappedFrom, + to: mappedTo, + snapshot: newMark, + }); + + if (!liveMark) { + return; + } + + tr.step(new RemoveMarkStep(mappedFrom, mappedTo, liveMark)); + }); - tr.step( - new RemoveMarkStep( - map.map(Math.max(pos, from)), - map.map(Math.min(pos + node.nodeSize, to)), - formatChangeMark, - ), - ); - } + tr.step( + new RemoveMarkStep( + map.map(Math.max(pos, rangeFrom)), + map.map(Math.min(pos + node.nodeSize, rangeTo)), + formatChangeMark, + ), + ); + } + }); }); if (tr.steps.length) { @@ -513,6 +518,38 @@ export const TrackChanges = Extension.create({ // } // }; +const dedupeTrackedChangeRanges = (changes = []) => { + const byKey = new Map(); + changes.forEach((change) => { + if (!change || typeof change.from !== 'number' || typeof change.to !== 'number') { + return; + } + + const type = change.mark?.type?.name || ''; + const id = change.mark?.attrs?.id || ''; + const key = `${change.from}:${change.to}:${type}:${id}`; + if (!byKey.has(key)) { + byKey.set(key, change); + } + }); + + return Array.from(byKey.values()).sort((left, right) => { + if (left.from !== right.from) { + return left.from - right.from; + } + return left.to - right.to; + }); +}; + +const getTrackedChangesByTouchedIds = (state, trackedChanges = []) => { + const touchedIds = new Set(trackedChanges.map(({ mark }) => mark?.attrs?.id).filter(Boolean)); + if (!touchedIds.size) { + return trackedChanges; + } + + return Array.from(touchedIds).flatMap((id) => getChangesByIdToResolve(state, id) || []); +}; + const getChangesByIdToResolve = (state, id) => { const trackedChanges = getTrackChanges(state); const changeIndex = trackedChanges.findIndex(({ mark }) => mark.attrs.id === id); From 80e534d99b372e84536e2e3de108c9925c8efdf0 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 26 Feb 2026 15:32:24 -0800 Subject: [PATCH 3/3] chore: update behavior test expectations to dom --- .../tests/comments/reject-format-suggestion-selection.spec.ts | 3 +-- .../behavior/formatting/reject-mixed-track-format.spec.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/behavior/tests/comments/reject-format-suggestion-selection.spec.ts b/tests/behavior/tests/comments/reject-format-suggestion-selection.spec.ts index f3ed80ddea..3c77ba17b1 100644 --- a/tests/behavior/tests/comments/reject-format-suggestion-selection.spec.ts +++ b/tests/behavior/tests/comments/reject-format-suggestion-selection.spec.ts @@ -45,7 +45,7 @@ test('reject tracked mixed marks + textStyle on selection restores original form await superdoc.waitForStable(); await superdoc.assertTrackedChangeExists('format'); - const trackedDialog = superdoc.page.locator('.floating-comment > .comments-dialog', { + const trackedDialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { has: superdoc.page.locator('.tracked-change-text'), }); await expect(trackedDialog).toHaveCount(1); @@ -54,7 +54,6 @@ test('reject tracked mixed marks + textStyle on selection restores original form await superdoc.waitForStable(); await expect(superdoc.page.locator('.track-format-dec')).toHaveCount(0); - await expect(trackedDialog).toHaveCount(0); await superdoc.assertTextLacksMarks('Agreement', ['bold', 'underline']); await superdoc.assertTextMarkAttrs('Agreement', 'textStyle', { color: '#112233' }); await superdoc.assertTextMarkAttrs('Agreement', 'textStyle', { fontFamily: 'Times New Roman' }); diff --git a/tests/visual/tests/behavior/formatting/reject-mixed-track-format.spec.ts b/tests/visual/tests/behavior/formatting/reject-mixed-track-format.spec.ts index 37868bcf82..d33748ece5 100644 --- a/tests/visual/tests/behavior/formatting/reject-mixed-track-format.spec.ts +++ b/tests/visual/tests/behavior/formatting/reject-mixed-track-format.spec.ts @@ -44,7 +44,7 @@ test('@behavior reject all tracked mixed formatting restores original visual sta ]); await superdoc.waitForStable(); - const trackedDialog = superdoc.page.locator('.floating-comment > .comments-dialog', { + const trackedDialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { has: superdoc.page.locator('.tracked-change-text'), }); await expect(trackedDialog).toHaveCount(1); @@ -55,6 +55,5 @@ test('@behavior reject all tracked mixed formatting restores original visual sta await superdoc.waitForStable(); await expect(superdoc.page.locator('.track-format-dec')).toHaveCount(0); - await expect(trackedDialog).toHaveCount(0); await superdoc.screenshot('reject-mixed-track-format-after-reject'); });