Skip to content

Commit 865eaf3

Browse files
authored
fix(track-changes): split independent replacement sidebar updates (#2886)
1 parent bc809f8 commit 865eaf3

3 files changed

Lines changed: 217 additions & 15 deletions

File tree

packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js

Lines changed: 88 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -877,22 +877,49 @@ const handleTrackedChangeTransaction = (trackedChangeMeta, trackedChanges, newEd
877877
}
878878

879879
const newTrackedChanges = { ...trackedChanges };
880-
let id = insertedMark?.attrs?.id || deletionMark?.attrs?.id || formatMark?.attrs?.id;
880+
const insertedId = insertedMark?.attrs?.id ?? null;
881+
const deletionId = deletionMark?.attrs?.id ?? null;
882+
const formatId = formatMark?.attrs?.id ?? null;
883+
const primaryId = insertedId || deletionId || formatId;
881884

882-
if (!id) {
885+
if (!primaryId) {
883886
return trackedChanges;
884887
}
885888

886-
// Maintain a map of tracked changes with their inserted/deleted ids
887-
let isNewChange = false;
888-
if (!newTrackedChanges[id]) {
889-
newTrackedChanges[id] = {};
890-
isNewChange = true;
891-
}
889+
const registerTrackedChangeId = (changeId, patch) => {
890+
if (!changeId) return false;
891+
892+
const existing = newTrackedChanges[changeId];
893+
if (existing) {
894+
Object.assign(existing, patch);
895+
return false;
896+
}
897+
898+
newTrackedChanges[changeId] = { ...patch };
899+
return true;
900+
};
901+
902+
const buildTrackedChangePayload = ({ event, marks, nodes, deletionNodes = [] }) => {
903+
if (!marks.insertedMark && !marks.deletionMark && !marks.formatMark) {
904+
return null;
905+
}
906+
907+
const trackedMarkId =
908+
marks.insertedMark?.attrs?.id ?? marks.deletionMark?.attrs?.id ?? marks.formatMark?.attrs?.id ?? null;
909+
if (!trackedMarkId) {
910+
return null;
911+
}
892912

893-
if (insertedMark) newTrackedChanges[id].insertion = id;
894-
if (deletionMark) newTrackedChanges[id].deletion = deletionMark.attrs?.id;
895-
if (formatMark) newTrackedChanges[id].format = formatMark.attrs?.id;
913+
return createOrUpdateTrackedChangeComment({
914+
documentId: editor.options.documentId,
915+
event,
916+
marks,
917+
deletionNodes,
918+
nodes,
919+
newEditorState,
920+
trackedChangesForId: getTrackChanges(newEditorState, trackedMarkId),
921+
});
922+
};
896923

897924
const { step } = trackedChangeMeta;
898925
let nodes = step?.slice?.content?.content || [];
@@ -909,9 +936,54 @@ const handleTrackedChangeTransaction = (trackedChangeMeta, trackedChanges, newEd
909936
}
910937

911938
const hasCandidateNodes = nodes.length > 0 || Boolean(deletionNodes?.length);
939+
const hasIndependentReplacementIds =
940+
Boolean(insertedMark && deletionMark) && Boolean(insertedId) && Boolean(deletionId) && insertedId !== deletionId;
941+
942+
if (hasIndependentReplacementIds) {
943+
const isNewInsertion = registerTrackedChangeId(insertedId, { insertion: insertedId });
944+
const isNewDeletion = registerTrackedChangeId(deletionId, { deletion: deletionId });
945+
946+
const insertionPayload = hasCandidateNodes
947+
? buildTrackedChangePayload({
948+
event: isNewInsertion ? 'add' : 'update',
949+
marks: {
950+
insertedMark,
951+
deletionMark: null,
952+
formatMark: null,
953+
},
954+
deletionNodes: [],
955+
nodes,
956+
})
957+
: null;
958+
959+
const deletionPayload =
960+
deletionMark && (hasCandidateNodes || getTrackChanges(newEditorState, deletionId).length > 0)
961+
? buildTrackedChangePayload({
962+
event: isNewDeletion ? 'add' : 'update',
963+
marks: {
964+
insertedMark: null,
965+
deletionMark,
966+
formatMark: null,
967+
},
968+
deletionNodes,
969+
nodes: [],
970+
})
971+
: null;
972+
973+
if (emitCommentEvent && insertionPayload) editor.emit('commentsUpdate', insertionPayload);
974+
if (emitCommentEvent && deletionPayload) editor.emit('commentsUpdate', deletionPayload);
975+
return newTrackedChanges;
976+
}
977+
978+
// Maintain a map of tracked changes with their inserted/deleted ids.
979+
const isNewChange = registerTrackedChangeId(primaryId, {
980+
...(insertedMark ? { insertion: primaryId } : {}),
981+
...(deletionMark ? { deletion: deletionId } : {}),
982+
...(formatMark ? { format: formatId } : {}),
983+
});
984+
912985
const emitParams = hasCandidateNodes
913-
? createOrUpdateTrackedChangeComment({
914-
documentId: editor.options.documentId,
986+
? buildTrackedChangePayload({
915987
event: isNewChange ? 'add' : 'update',
916988
marks: {
917989
insertedMark,
@@ -920,7 +992,6 @@ const handleTrackedChangeTransaction = (trackedChangeMeta, trackedChanges, newEd
920992
},
921993
deletionNodes,
922994
nodes,
923-
newEditorState,
924995
})
925996
: null;
926997

@@ -1064,7 +1135,9 @@ const createOrUpdateTrackedChangeComment = ({
10641135
const { author, authorEmail, authorImage, date, importedAuthor } = attrs;
10651136
const id = attrs.id;
10661137

1067-
let isReplacement = !!(marks.insertedMark && marks.deletionMark);
1138+
const insertedMarkId = marks.insertedMark?.attrs?.id ?? null;
1139+
const deletionMarkId = marks.deletionMark?.attrs?.id ?? null;
1140+
let isReplacement = Boolean(insertedMarkId && deletionMarkId && insertedMarkId === deletionMarkId);
10681141

10691142
// Fallback: check the document for both mark types under the same ID
10701143
// (covers edge cases where transaction meta only carries one mark)

packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.test.js

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -994,6 +994,65 @@ describe('internal helper functions', () => {
994994
);
995995
});
996996

997+
it('handleTrackedChangeTransaction emits separate events for independent replacements', () => {
998+
const schema = createCommentSchema();
999+
const insertMark = schema.marks[TrackInsertMarkName].create({
1000+
id: 'change-insert-only',
1001+
author: 'Alice',
1002+
authorEmail: 'alice@example.com',
1003+
date: 'today',
1004+
});
1005+
const deleteMark = schema.marks[TrackDeleteMarkName].create({
1006+
id: 'change-delete-only',
1007+
author: 'Alice',
1008+
authorEmail: 'alice@example.com',
1009+
date: 'today',
1010+
});
1011+
1012+
const deletedNode = schema.text('Removed', [deleteMark]);
1013+
const insertedNode = schema.text('Added', [insertMark]);
1014+
const paragraph = schema.node('paragraph', null, [deletedNode, insertedNode]);
1015+
const doc = schema.node('doc', null, [paragraph]);
1016+
const state = EditorState.create({ schema, doc });
1017+
const editor = { options: { documentId: 'doc-1' }, emit: vi.fn() };
1018+
1019+
const meta = {
1020+
insertedMark: insertMark,
1021+
deletionMark: deleteMark,
1022+
formatMark: null,
1023+
deletionNodes: [deletedNode],
1024+
step: { slice: { content: { content: [insertedNode] } } },
1025+
};
1026+
1027+
const trackedChanges = handleTrackedChangeTransaction(meta, {}, state, editor);
1028+
1029+
expect(trackedChanges['change-insert-only']).toMatchObject({ insertion: 'change-insert-only' });
1030+
expect(trackedChanges['change-delete-only']).toMatchObject({ deletion: 'change-delete-only' });
1031+
expect(editor.emit).toHaveBeenCalledTimes(2);
1032+
expect(editor.emit).toHaveBeenNthCalledWith(
1033+
1,
1034+
'commentsUpdate',
1035+
expect.objectContaining({
1036+
event: comments_module_events.ADD,
1037+
changeId: 'change-insert-only',
1038+
trackedChangeType: TrackInsertMarkName,
1039+
trackedChangeText: 'Added',
1040+
deletedText: null,
1041+
}),
1042+
);
1043+
expect(editor.emit).toHaveBeenNthCalledWith(
1044+
2,
1045+
'commentsUpdate',
1046+
expect.objectContaining({
1047+
event: comments_module_events.ADD,
1048+
changeId: 'change-delete-only',
1049+
trackedChangeType: TrackDeleteMarkName,
1050+
trackedChangeText: '',
1051+
deletedText: 'Removed',
1052+
}),
1053+
);
1054+
});
1055+
9971056
it('handleTrackedChangeTransaction returns original state when no marks provided', () => {
9981057
const schema = createCommentSchema();
9991058
const doc = schema.node('doc', null, [schema.node('paragraph', null, [schema.text('Text')])]);
@@ -1167,6 +1226,42 @@ describe('internal helper functions', () => {
11671226
expect(payload?.trackedChangeDisplayType).toBeNull();
11681227
});
11691228

1229+
it('does not collapse distinct insert/delete ids into one replacement payload', () => {
1230+
const schema = createCommentSchema();
1231+
const insertMark = schema.marks[TrackInsertMarkName].create({
1232+
id: 'replace-insert-1',
1233+
author: 'Author',
1234+
authorEmail: 'author@example.com',
1235+
date: 'today',
1236+
});
1237+
const deleteMark = schema.marks[TrackDeleteMarkName].create({
1238+
id: 'replace-delete-1',
1239+
author: 'Author',
1240+
authorEmail: 'author@example.com',
1241+
date: 'today',
1242+
});
1243+
1244+
const docInsertNode = schema.text('replacement', [insertMark]);
1245+
const docDeleteNode = schema.text('original', [deleteMark]);
1246+
const doc = schema.node('doc', null, [schema.node('paragraph', null, [docDeleteNode, docInsertNode])]);
1247+
const state = EditorState.create({ schema, doc });
1248+
1249+
const payload = createOrUpdateTrackedChangeComment({
1250+
event: 'add',
1251+
marks: { insertedMark: insertMark, deletionMark: deleteMark, formatMark: null },
1252+
deletionNodes: [docDeleteNode],
1253+
nodes: [docInsertNode],
1254+
newEditorState: state,
1255+
documentId: 'doc-1',
1256+
trackedChangesForId: [{ mark: insertMark, from: 1, to: doc.content.size }],
1257+
});
1258+
1259+
expect(payload?.changeId).toBe('replace-insert-1');
1260+
expect(payload?.trackedChangeType).toBe(TrackInsertMarkName);
1261+
expect(payload?.trackedChangeText).toBe('replacement');
1262+
expect(payload?.deletedText).toBe('');
1263+
});
1264+
11701265
it('createOrUpdateTrackedChangeComment builds add and update payloads', () => {
11711266
const schema = createCommentSchema();
11721267
const insertMark = schema.marks[TrackInsertMarkName].create({

tests/behavior/tests/comments/tracked-change-independent-replacement.spec.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,40 @@ test.describe("trackedChanges.replacements='independent'", () => {
8080
expect(uniqueIds.size).toBe(allIds.length);
8181
});
8282

83+
test('body replacement sidebar shows separate added and deleted bubbles', async ({ superdoc }) => {
84+
await assertDocumentApiReady(superdoc.page);
85+
86+
await superdoc.type('Replace ME now');
87+
await superdoc.waitForStable();
88+
await superdoc.setDocumentMode('suggesting');
89+
await superdoc.waitForStable();
90+
91+
await superdoc.tripleClickLine(0);
92+
await superdoc.waitForStable();
93+
await superdoc.type('Replace it now');
94+
await superdoc.waitForStable();
95+
96+
await expect.poll(async () => (await listTrackChanges(superdoc.page)).total).toBeGreaterThanOrEqual(2);
97+
98+
const dialogs = superdoc.page.locator('.comment-placeholder .comments-dialog', {
99+
has: superdoc.page.locator('.tracked-change-text'),
100+
});
101+
await expect(dialogs).toHaveCount(2);
102+
await expect(
103+
superdoc.page.locator('.comment-placeholder .comments-dialog .change-type', { hasText: 'Replaced' }),
104+
).toHaveCount(0);
105+
106+
const deletedDialog = superdoc.page.locator('.comment-placeholder .comments-dialog', {
107+
has: superdoc.page.locator('.tracked-change-text.is-deleted', { hasText: 'ME' }),
108+
});
109+
const insertedDialog = superdoc.page.locator('.comment-placeholder .comments-dialog', {
110+
has: superdoc.page.locator('.tracked-change-text.is-inserted', { hasText: 'it' }),
111+
});
112+
113+
await expect(deletedDialog).toHaveCount(1);
114+
await expect(insertedDialog).toHaveCount(1);
115+
});
116+
83117
test('accepting the insertion leaves the deletion addressable on its own', async ({ superdoc }) => {
84118
await assertDocumentApiReady(superdoc.page);
85119

0 commit comments

Comments
 (0)