From 9ef1b2bab0f58b1fc1090741e2bb9475fc117446 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 3 Mar 2026 14:16:08 -0300 Subject: [PATCH 1/2] fix(track-changes): show correct format description when adding marks to formatted text (SD-2077) When applying formatting (e.g. bold) to text that already had different formatting (e.g. highlight) in suggesting mode, the tracked change comment incorrectly stated the original formatting was removed. Root cause: addMarkStep populated the `before` array with ALL existing marks on the text, rather than only marks of the same type being replaced. The display logic diffs `before` vs `after` to generate the description, so unrelated marks in `before` appeared as removals. Fix: only capture an existing mark of the same type as the step mark into `before` (for replacement cases like changing textStyle color). For pure additions, `before` is now empty. This matches the pattern already used by removeMarkStep. --- .../trackChangesHelpers/addMarkStep.js | 14 +- .../trackChangesHelpers.test.js | 47 ++++++ .../sd-2077-format-change-display.spec.ts | 135 ++++++++++++++++++ 3 files changed, 190 insertions(+), 6 deletions(-) create mode 100644 tests/behavior/tests/comments/sd-2077-format-change-display.spec.ts diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js index 57a5f16e5e..2b56b4e2ef 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js @@ -70,12 +70,14 @@ export const addMarkStep = ({ state, step, newTr, doc, user, date }) => { }); } } else { - before = liveMarks - .filter((mark) => ![TrackDeleteMarkName, TrackFormatMarkName].includes(mark.type.name)) - .map((mark) => ({ - type: mark.type.name, - attrs: { ...mark.attrs }, - })); + const existingMarkOfSameType = liveMarks.find( + (mark) => + mark.type.name === step.mark.type.name && + ![TrackDeleteMarkName, TrackFormatMarkName].includes(mark.type.name), + ); + before = existingMarkOfSameType + ? [{ type: existingMarkOfSameType.type.name, attrs: { ...existingMarkOfSameType.attrs } }] + : []; after = [ { diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js index 170d1f9357..eab5105d9f 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js @@ -399,6 +399,53 @@ describe('trackChangesHelpers', () => { expect(meta?.formatMark?.attrs?.after).toEqual([{ type: 'highlight', attrs: { color: '#E4668C' } }]); }); + it('addMarkStep does not include unrelated marks in before (SD-2077)', () => { + const highlight = schema.marks.highlight.create({ color: '#FFFF00' }); + const doc = createDocWithText('Hello', [highlight]); + const state = createState(doc); + const boldMark = schema.marks.bold.create(); + const step = new AddMarkStep(1, 6, boldMark); + const newTr = state.tr; + + addMarkStep({ + state, + step, + newTr, + doc: state.doc, + user, + date, + }); + + const meta = newTr.getMeta(TrackChangesBasePluginKey); + expect(meta?.formatMark?.type.name).toBe(TrackFormatMarkName); + expect(meta?.formatMark?.attrs?.before).toEqual([]); + expect(meta?.formatMark?.attrs?.after).toEqual([{ type: 'bold', attrs: boldMark.attrs }]); + }); + + it('addMarkStep only captures same-type mark in before when replacing (SD-2077)', () => { + const highlight = schema.marks.highlight.create({ color: '#FFFF00' }); + const textStyle = schema.marks.textStyle.create({ color: '#112233', fontSize: '11pt' }); + const doc = createDocWithText('Hello', [highlight, textStyle]); + const state = createState(doc); + const changedTextStyle = schema.marks.textStyle.create({ color: '#FF0000', fontSize: '11pt' }); + const step = new AddMarkStep(1, 6, changedTextStyle); + const newTr = state.tr; + + addMarkStep({ + state, + step, + newTr, + doc: state.doc, + user, + date, + }); + + const meta = newTr.getMeta(TrackChangesBasePluginKey); + expect(meta?.formatMark?.type.name).toBe(TrackFormatMarkName); + expect(meta?.formatMark?.attrs?.before).toEqual([{ type: 'textStyle', attrs: textStyle.attrs }]); + expect(meta?.formatMark?.attrs?.after).toEqual([{ type: 'textStyle', attrs: changedTextStyle.attrs }]); + }); + it('removeMarkStep records previous formatting when mark removed', () => { const bold = schema.marks.bold.create(); const doc = createDocWithText('Styled', [bold]); diff --git a/tests/behavior/tests/comments/sd-2077-format-change-display.spec.ts b/tests/behavior/tests/comments/sd-2077-format-change-display.spec.ts new file mode 100644 index 0000000000..600ebbbf76 --- /dev/null +++ b/tests/behavior/tests/comments/sd-2077-format-change-display.spec.ts @@ -0,0 +1,135 @@ +import type { Page } from '@playwright/test'; +import { test, expect } from '../../fixtures/superdoc.js'; +import { assertDocumentApiReady, listTrackChanges } from '../../helpers/document-api.js'; + +test.use({ config: { toolbar: 'full', comments: 'on', trackChanges: true } }); + +type EditorCommand = [name: string, ...args: unknown[]]; + +async function runCommands(page: Page, commands: EditorCommand[]): Promise { + for (const [name, ...args] of commands) { + await page.evaluate(({ name, args }) => (window as any).editor.commands[name](...args), { name, args }); + } +} + +test.describe('SD-2077 tracked format change displays correct description', () => { + test('adding bold to highlighted text shows only bold addition', async ({ superdoc }) => { + await assertDocumentApiReady(superdoc.page); + + // Type text and apply highlight in editing mode + await superdoc.type('Hello world'); + await superdoc.waitForStable(); + await superdoc.selectAll(); + await runCommands(superdoc.page, [['setHighlight', '#FFFF00']]); + await superdoc.waitForStable(); + + // Switch to suggesting mode + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + // Select text and apply bold + await superdoc.selectAll(); + await superdoc.bold(); + await superdoc.waitForStable(); + + // Verify tracked format change exists + await superdoc.assertTrackedChangeExists('format'); + + // Wait for the tracked change comment dialog to appear + const dialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { + has: superdoc.page.locator('.tracked-change-text'), + }); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + + // The format description should mention bold but NOT mention highlight removal + const formatText = dialog.locator('.tracked-change-text'); + await expect(formatText).toContainText('bold'); + + const text = await formatText.textContent(); + expect(text).not.toContain('removed'); + expect(text).not.toContain('highlight'); + + await superdoc.snapshot('sd-2077-bold-on-highlighted'); + }); + + test('adding italic to bold text shows only italic addition', async ({ superdoc }) => { + await assertDocumentApiReady(superdoc.page); + + // Type text and apply bold in editing mode + await superdoc.type('Hello world'); + await superdoc.waitForStable(); + await superdoc.selectAll(); + await superdoc.bold(); + await superdoc.waitForStable(); + + // Verify bold is applied + await superdoc.assertTextHasMarks('Hello', ['bold']); + + // Switch to suggesting mode + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + // Select text and apply italic + await superdoc.selectAll(); + await superdoc.italic(); + await superdoc.waitForStable(); + + // Verify tracked format change exists + await superdoc.assertTrackedChangeExists('format'); + + const dialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { + has: superdoc.page.locator('.tracked-change-text'), + }); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + + // Should show italic addition, not bold removal + const formatText = dialog.locator('.tracked-change-text'); + await expect(formatText).toContainText('italic'); + + const text = await formatText.textContent(); + expect(text).not.toContain('removed'); + expect(text).not.toContain('bold'); + + await superdoc.snapshot('sd-2077-italic-on-bold'); + }); + + test('changing color on highlighted text shows only color change', async ({ superdoc }) => { + await assertDocumentApiReady(superdoc.page); + + // Type text and apply highlight + color in editing mode + await superdoc.type('Hello world'); + await superdoc.waitForStable(); + await superdoc.selectAll(); + await runCommands(superdoc.page, [ + ['setHighlight', '#FFFF00'], + ['setColor', '#112233'], + ]); + await superdoc.waitForStable(); + + // Switch to suggesting mode + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + // Select text and change color + await superdoc.selectAll(); + await runCommands(superdoc.page, [['setColor', '#FF0000']]); + await superdoc.waitForStable(); + + // Verify tracked format change exists + await superdoc.assertTrackedChangeExists('format'); + + const dialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { + has: superdoc.page.locator('.tracked-change-text'), + }); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + + // Should show color change, not highlight removal + const formatText = dialog.locator('.tracked-change-text'); + await expect(formatText).toContainText('color'); + + const text = await formatText.textContent(); + expect(text).not.toContain('removed highlight'); + + await superdoc.snapshot('sd-2077-color-on-highlighted'); + }); +}); From de42e1facfdb7ee63d90298afcd4236caf7a7342 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 3 Mar 2026 13:18:39 -0800 Subject: [PATCH 2/2] chore: review comments --- .../sd-2077-format-change-display.spec.ts | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/behavior/tests/comments/sd-2077-format-change-display.spec.ts b/tests/behavior/tests/comments/sd-2077-format-change-display.spec.ts index 600ebbbf76..ef72631087 100644 --- a/tests/behavior/tests/comments/sd-2077-format-change-display.spec.ts +++ b/tests/behavior/tests/comments/sd-2077-format-change-display.spec.ts @@ -1,6 +1,6 @@ import type { Page } from '@playwright/test'; import { test, expect } from '../../fixtures/superdoc.js'; -import { assertDocumentApiReady, listTrackChanges } from '../../helpers/document-api.js'; +import { assertDocumentApiReady } from '../../helpers/document-api.js'; test.use({ config: { toolbar: 'full', comments: 'on', trackChanges: true } }); @@ -12,6 +12,14 @@ async function runCommands(page: Page, commands: EditorCommand[]): Promise } } +async function expectTrackedFormatDialog(page: Page) { + const dialog = page.locator('.comment-placeholder .comments-dialog', { + has: page.locator('.tracked-change-text'), + }); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + return dialog; +} + test.describe('SD-2077 tracked format change displays correct description', () => { test('adding bold to highlighted text shows only bold addition', async ({ superdoc }) => { await assertDocumentApiReady(superdoc.page); @@ -36,10 +44,7 @@ test.describe('SD-2077 tracked format change displays correct description', () = await superdoc.assertTrackedChangeExists('format'); // Wait for the tracked change comment dialog to appear - const dialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { - has: superdoc.page.locator('.tracked-change-text'), - }); - await expect(dialog).toBeVisible({ timeout: 10_000 }); + const dialog = await expectTrackedFormatDialog(superdoc.page); // The format description should mention bold but NOT mention highlight removal const formatText = dialog.locator('.tracked-change-text'); @@ -77,10 +82,7 @@ test.describe('SD-2077 tracked format change displays correct description', () = // Verify tracked format change exists await superdoc.assertTrackedChangeExists('format'); - const dialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { - has: superdoc.page.locator('.tracked-change-text'), - }); - await expect(dialog).toBeVisible({ timeout: 10_000 }); + const dialog = await expectTrackedFormatDialog(superdoc.page); // Should show italic addition, not bold removal const formatText = dialog.locator('.tracked-change-text'); @@ -118,10 +120,7 @@ test.describe('SD-2077 tracked format change displays correct description', () = // Verify tracked format change exists await superdoc.assertTrackedChangeExists('format'); - const dialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { - has: superdoc.page.locator('.tracked-change-text'), - }); - await expect(dialog).toBeVisible({ timeout: 10_000 }); + const dialog = await expectTrackedFormatDialog(superdoc.page); // Should show color change, not highlight removal const formatText = dialog.locator('.tracked-change-text');