Skip to content

Commit a09d2a6

Browse files
committed
fix(track-changes): cancel tracked format changes when reverted to original (SD-2181)
When a format change (e.g., superscript) is applied in track changes mode and then reverted (e.g., back to baseline), the tracked change now cancels out instead of persisting as two separate changes with zero net effect. - Filter reverted entries from the `after` array in the `foundBefore` branch - Add `isTrackFormatNoOp` check to detect and remove no-op format changes - Handle identity attribute values (vertAlign: baseline, position: 0pt) - Compose `normalizeSnapshotAttrs` on `normalizeAttrs` to avoid duplication - Export and reuse `getTypeName` helper across modules
1 parent c76e9e8 commit a09d2a6

4 files changed

Lines changed: 232 additions & 6 deletions

File tree

packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@ import { TrackDeleteMarkName, TrackFormatMarkName } from '../constants.js';
22
import { v4 as uuidv4 } from 'uuid';
33
import { TrackChangesBasePluginKey } from '../plugins/trackChangesBasePlugin.js';
44
import { CommentsPluginKey } from '../../comment/comments-plugin.js';
5-
import { hasMatchingMark, markSnapshotMatchesStepMark, upsertMarkSnapshotByType } from './markSnapshotHelpers.js';
5+
import {
6+
hasMatchingMark,
7+
markSnapshotMatchesStepMark,
8+
upsertMarkSnapshotByType,
9+
isTrackFormatNoOp,
10+
getTypeName,
11+
} from './markSnapshotHelpers.js';
612
import { getLiveInlineMarksInRange } from './getLiveInlineMarksInRange.js';
713

814
/**
@@ -61,7 +67,9 @@ export const addMarkStep = ({ state, step, newTr, doc, user, date }) => {
6167
before = [
6268
...formatChangeMark.attrs.before.filter((mark) => !markSnapshotMatchesStepMark(mark, step.mark, true)),
6369
];
64-
after = [...formatChangeMark.attrs.after];
70+
// The step restores the original mark for this type — remove the
71+
// corresponding "after" entry since the change has been reverted.
72+
after = formatChangeMark.attrs.after.filter((mark) => getTypeName(mark) !== step.mark.type.name);
6573
} else {
6674
before = [...formatChangeMark.attrs.before];
6775
after = upsertMarkSnapshotByType(formatChangeMark.attrs.after, {
@@ -87,6 +95,15 @@ export const addMarkStep = ({ state, step, newTr, doc, user, date }) => {
8795
];
8896
}
8997

98+
// Check if the format change is effectively a no-op (e.g., reverting
99+
// vertAlign to 'baseline' when the original had no vertAlign).
100+
if (isTrackFormatNoOp(before, after)) {
101+
if (formatChangeMark) {
102+
newTr.removeMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), formatChangeMark);
103+
}
104+
return;
105+
}
106+
90107
if (after.length || before.length) {
91108
const newFormatMark = state.schema.marks[TrackFormatMarkName].create({
92109
id: wid,

packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markSnapshotHelpers.js

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,71 @@ const normalizeAttrs = (attrs = {}) => {
44
return Object.fromEntries(Object.entries(attrs).filter(([, value]) => value !== null && value !== undefined));
55
};
66

7+
/**
8+
* Attribute values that are semantically equivalent to "not set" for tracking purposes.
9+
* These represent the default visual state and should not count as a change.
10+
*/
11+
const IDENTITY_ATTR_VALUES = {
12+
vertAlign: 'baseline',
13+
position: '0pt',
14+
};
15+
16+
/**
17+
* Mark types where the mark's effect is determined entirely by its attributes.
18+
* An entry with empty normalized attrs means the mark has no visual effect.
19+
* In contrast, structural marks (bold, italic) have their effect from being present.
20+
*/
21+
const ATTRIBUTE_ONLY_MARKS = ['textStyle'];
22+
23+
/**
24+
* Normalize snapshot attrs for tracked change comparison.
25+
* Strips null/undefined AND identity values that represent the default visual state.
26+
*/
27+
const normalizeSnapshotAttrs = (attrs = {}) => {
28+
const base = normalizeAttrs(attrs);
29+
return Object.fromEntries(Object.entries(base).filter(([key, value]) => IDENTITY_ATTR_VALUES[key] !== value));
30+
};
31+
32+
export const getTypeName = (markLike) => {
33+
return markLike?.type?.name ?? markLike?.type;
34+
};
35+
36+
/**
37+
* Check if a tracked format change is effectively a no-op.
38+
* Compares before and after snapshots after normalizing identity attribute values.
39+
* A no-op means the format change has no net visual effect.
40+
*/
41+
export const isTrackFormatNoOp = (before, after) => {
42+
const normalize = (entries) =>
43+
entries
44+
.map((s) => ({
45+
type: getTypeName(s),
46+
attrs: normalizeSnapshotAttrs(s.attrs || {}),
47+
}))
48+
.filter((s) => {
49+
// For attribute-only marks (e.g. textStyle), empty attrs = no visual effect → filter out
50+
if (ATTRIBUTE_ONLY_MARKS.includes(s.type) && Object.keys(s.attrs).length === 0) return false;
51+
return true;
52+
});
53+
54+
const normBefore = normalize(before);
55+
const normAfter = normalize(after);
56+
57+
if (normBefore.length === 0 && normAfter.length === 0) return true;
58+
if (normBefore.length !== normAfter.length) return false;
59+
60+
return (
61+
normBefore.every((b) => normAfter.some((a) => a.type === b.type && isEqual(a.attrs, b.attrs))) &&
62+
normAfter.every((a) => normBefore.some((b) => b.type === a.type && isEqual(b.attrs, a.attrs)))
63+
);
64+
};
65+
766
export const attrsExactlyMatch = (left = {}, right = {}) => {
867
const normalizedLeft = normalizeAttrs(left);
968
const normalizedRight = normalizeAttrs(right);
1069
return isEqual(normalizedLeft, normalizedRight);
1170
};
1271

13-
const getTypeName = (markLike) => {
14-
return markLike?.type?.name ?? markLike?.type;
15-
};
16-
1772
const marksMatch = (left, right, exact = true) => {
1873
if (!left || !right || getTypeName(left) !== getTypeName(right)) {
1974
return false;

packages/super-editor/src/extensions/track-changes/trackChangesHelpers/markSnapshotHelpers.test.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
hasMatchingMark,
88
upsertMarkSnapshotByType,
99
findMarkInRangeBySnapshot,
10+
isTrackFormatNoOp,
1011
} from './markSnapshotHelpers.js';
1112

1213
describe('markSnapshotHelpers', () => {
@@ -140,6 +141,69 @@ describe('markSnapshotHelpers', () => {
140141
expect(match).toBeNull();
141142
});
142143

144+
describe('isTrackFormatNoOp', () => {
145+
it('returns true when both before and after are empty', () => {
146+
expect(isTrackFormatNoOp([], [])).toBe(true);
147+
});
148+
149+
it('returns true when after has only textStyle with vertAlign baseline (identity value)', () => {
150+
// Scenario: text had no textStyle, user added superscript then reverted to baseline
151+
expect(isTrackFormatNoOp([], [{ type: 'textStyle', attrs: { vertAlign: 'baseline' } }])).toBe(true);
152+
});
153+
154+
it('returns true when before and after differ only by vertAlign baseline', () => {
155+
// Scenario: text had textStyle with fontSize, user added superscript then reverted
156+
expect(
157+
isTrackFormatNoOp(
158+
[{ type: 'textStyle', attrs: { fontSize: '24pt' } }],
159+
[{ type: 'textStyle', attrs: { fontSize: '24pt', vertAlign: 'baseline' } }],
160+
),
161+
).toBe(true);
162+
});
163+
164+
it('returns false for a real format change', () => {
165+
expect(
166+
isTrackFormatNoOp(
167+
[{ type: 'textStyle', attrs: { fontSize: '12pt' } }],
168+
[{ type: 'textStyle', attrs: { fontSize: '24pt' } }],
169+
),
170+
).toBe(false);
171+
});
172+
173+
it('returns false when bold is added (structural mark)', () => {
174+
expect(isTrackFormatNoOp([], [{ type: 'bold', attrs: {} }])).toBe(false);
175+
});
176+
177+
it('returns false when bold is removed (structural mark)', () => {
178+
expect(isTrackFormatNoOp([{ type: 'bold', attrs: {} }], [])).toBe(false);
179+
});
180+
181+
it('returns true when textStyle with only null attrs is in after', () => {
182+
expect(isTrackFormatNoOp([], [{ type: 'textStyle', attrs: { vertAlign: null, position: null } }])).toBe(true);
183+
});
184+
185+
it('returns false when non-identity textStyle change exists alongside baseline revert', () => {
186+
// Bold was also changed — not a no-op
187+
expect(
188+
isTrackFormatNoOp([{ type: 'bold', attrs: {} }], [{ type: 'textStyle', attrs: { vertAlign: 'baseline' } }]),
189+
).toBe(false);
190+
});
191+
192+
it('returns true when position is reverted to 0pt (identity value)', () => {
193+
expect(isTrackFormatNoOp([], [{ type: 'textStyle', attrs: { position: '0pt' } }])).toBe(true);
194+
});
195+
196+
it('returns true when vertAlign superscript matches in both before and after', () => {
197+
// Both say the same thing — no net change
198+
expect(
199+
isTrackFormatNoOp(
200+
[{ type: 'textStyle', attrs: { vertAlign: 'superscript' } }],
201+
[{ type: 'textStyle', attrs: { vertAlign: 'superscript' } }],
202+
),
203+
).toBe(true);
204+
});
205+
});
206+
143207
it('findMarkInRangeBySnapshot falls back to subset attr match for sparse snapshots', () => {
144208
const richTextStyle = schema.marks.textStyle.create({
145209
styleId: 'Emphasis',

packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,96 @@ describe('trackChangesHelpers', () => {
446446
expect(meta?.formatMark?.attrs?.after).toEqual([{ type: 'textStyle', attrs: changedTextStyle.attrs }]);
447447
});
448448

449+
it('addMarkStep removes trackFormat when reverting to original state (SD-2181)', () => {
450+
// Step 1: Plain text, apply superscript → creates trackFormat
451+
const state = createState(createDocWithText('Hello'));
452+
const superscriptMark = schema.marks.textStyle.create({ vertAlign: 'superscript' });
453+
const step1 = new AddMarkStep(2, 6, superscriptMark);
454+
const newTr1 = state.tr;
455+
456+
addMarkStep({
457+
state,
458+
step: step1,
459+
newTr: newTr1,
460+
doc: state.doc,
461+
user,
462+
date,
463+
});
464+
465+
const meta1 = newTr1.getMeta(TrackChangesBasePluginKey);
466+
expect(meta1?.formatMark?.type.name).toBe(TrackFormatMarkName);
467+
468+
// Step 2: Apply baseline (revert) on the tracked state
469+
const state2 = state.apply(newTr1);
470+
const baselineMark = schema.marks.textStyle.create({ vertAlign: 'baseline' });
471+
const step2 = new AddMarkStep(2, 6, baselineMark);
472+
const newTr2 = state2.tr;
473+
474+
addMarkStep({
475+
state: state2,
476+
step: step2,
477+
newTr: newTr2,
478+
doc: state2.doc,
479+
user,
480+
date,
481+
});
482+
483+
// The trackFormat mark should be removed (no-op), no metadata set
484+
const meta2 = newTr2.getMeta(TrackChangesBasePluginKey);
485+
expect(meta2).toBeUndefined();
486+
487+
// Verify no trackFormat mark remains in the document
488+
const finalState = state2.apply(newTr2);
489+
let hasTrackFormat = false;
490+
finalState.doc.descendants((node) => {
491+
if (node.marks?.some((m) => m.type.name === TrackFormatMarkName)) {
492+
hasTrackFormat = true;
493+
}
494+
});
495+
expect(hasTrackFormat).toBe(false);
496+
});
497+
498+
it('addMarkStep preserves other tracked types when partially reverting (SD-2181)', () => {
499+
// Step 1: Apply bold on plain text → creates trackFormat with after: [bold]
500+
const state = createState(createDocWithText('Hello'));
501+
const boldMark = schema.marks.bold.create();
502+
const step1 = new AddMarkStep(2, 6, boldMark);
503+
const newTr1 = state.tr;
504+
505+
addMarkStep({
506+
state,
507+
step: step1,
508+
newTr: newTr1,
509+
doc: state.doc,
510+
user,
511+
date,
512+
});
513+
514+
const state2 = state.apply(newTr1);
515+
516+
// Step 2: Also change textStyle color → trackFormat now has after: [bold, textStyle]
517+
const colorMark = schema.marks.textStyle.create({ color: '#FF0000' });
518+
const step2 = new AddMarkStep(2, 6, colorMark);
519+
const newTr2 = state2.tr;
520+
521+
addMarkStep({
522+
state: state2,
523+
step: step2,
524+
newTr: newTr2,
525+
doc: state2.doc,
526+
user,
527+
date,
528+
});
529+
530+
const meta2 = newTr2.getMeta(TrackChangesBasePluginKey);
531+
expect(meta2?.formatMark?.attrs?.after).toEqual(
532+
expect.arrayContaining([
533+
expect.objectContaining({ type: 'bold' }),
534+
expect.objectContaining({ type: 'textStyle' }),
535+
]),
536+
);
537+
});
538+
449539
it('removeMarkStep records previous formatting when mark removed', () => {
450540
const bold = schema.marks.bold.create();
451541
const doc = createDocWithText('Styled', [bold]);

0 commit comments

Comments
 (0)