Skip to content

Commit bc7cba3

Browse files
fix(tracked-changes): undo/redo applies to both document and comment bubbles (#2437)
* feat(comments): enhance undo/redo functionality for tracked changes - Added functionality to resync tracked-change threads during undo/redo transactions in SuperDoc. - Updated the comments store to reopen resolved tracked change comments when updates occur, ensuring they remain actionable. - Introduced tests to verify the correct behavior of tracked changes during undo/redo operations, maintaining sidebar bubble visibility and state consistency. * feat(comments): implement event emission for deleted tracked-change comments * feat(comments): enhance comment handling for tracked changes on import * fix(comments-store): preserve tracked-change thread state during undo/redo operations * test(comments-store): add unit tests for resolved TC pruning and snapshot restore - Test that already-resolved tracked-change comments survive empty replay sync (the `if (comment.resolvedTime) return true` guard in pruneStaleTrackedChangeComments) - Test that snapshot restore works end-to-end: undo reopens thread, redo restores resolved state from WeakMap snapshot instead of hard-deleting * fix(comments-store): emit UPDATE event when snapshot restores resolved state When redo re-resolves a thread via the WeakMap snapshot, collaborators were never notified — undo broadcast "reopened" but redo silently re-resolved. Now emits an UPDATE event after snapshot restore so collab peers stay in sync. --------- Co-authored-by: Caio Pizzol <caio@harbourshare.com>
1 parent ebef7f8 commit bc7cba3

8 files changed

Lines changed: 664 additions & 12 deletions

File tree

packages/superdoc/src/SuperDoc.test.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ const buildCommentsStore = () => ({
177177
handleEditorLocationsUpdate: vi.fn(),
178178
clearEditorCommentPositions: vi.fn(),
179179
handleTrackedChangeUpdate: vi.fn(),
180+
syncTrackedChangePositionsWithDocument: vi.fn(),
180181
syncTrackedChangeComments: vi.fn(),
181182
removePendingComment: vi.fn(),
182183
setActiveComment: vi.fn(),
@@ -519,6 +520,52 @@ describe('SuperDoc.vue', () => {
519520
expect(commentsStoreStub.syncTrackedChangeComments).not.toHaveBeenCalled();
520521
});
521522

523+
it('resyncs tracked-change threads on undo/redo transactions', async () => {
524+
const superdocStub = createSuperdocStub();
525+
const wrapper = await mountComponent(superdocStub);
526+
await nextTick();
527+
528+
const options = wrapper.findComponent(SuperEditorStub).props('options');
529+
const editorMock = { options: { documentId: 'doc-1' } };
530+
531+
const makeTransaction = (inputType) => ({
532+
getMeta: vi.fn((key) => (key === 'inputType' ? inputType : undefined)),
533+
});
534+
535+
options.onTransaction({
536+
editor: editorMock,
537+
transaction: makeTransaction('historyUndo'),
538+
duration: 4,
539+
});
540+
541+
expect(commentsStoreStub.syncTrackedChangePositionsWithDocument).toHaveBeenCalledWith({
542+
documentId: 'doc-1',
543+
editor: editorMock,
544+
});
545+
expect(commentsStoreStub.syncTrackedChangeComments).toHaveBeenCalledWith({
546+
superdoc: superdocStub,
547+
editor: editorMock,
548+
});
549+
550+
commentsStoreStub.syncTrackedChangePositionsWithDocument.mockClear();
551+
commentsStoreStub.syncTrackedChangeComments.mockClear();
552+
553+
options.onTransaction({
554+
editor: editorMock,
555+
transaction: makeTransaction('historyRedo'),
556+
duration: 5,
557+
});
558+
559+
expect(commentsStoreStub.syncTrackedChangePositionsWithDocument).toHaveBeenCalledWith({
560+
documentId: 'doc-1',
561+
editor: editorMock,
562+
});
563+
expect(commentsStoreStub.syncTrackedChangeComments).toHaveBeenCalledWith({
564+
superdoc: superdocStub,
565+
editor: editorMock,
566+
});
567+
});
568+
522569
it('reconciles replay updates by importedId before commentId to avoid duplicate comments', async () => {
523570
const superdocStub = createSuperdocStub();
524571
const wrapper = await mountComponent(superdocStub);

packages/superdoc/src/SuperDoc.vue

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,7 @@ const onEditorTransaction = (payload = {}) => {
996996
if (inputType === 'historyUndo' || inputType === 'historyRedo') {
997997
const documentId = editor?.options?.documentId;
998998
syncTrackedChangePositionsWithDocument({ documentId, editor });
999+
syncTrackedChangeComments({ superdoc: proxy.$superdoc, editor });
9991000
}
10001001
10011002
emitEditorTransaction(buildEditorTransactionPayload(payload));

packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ const mountDialog = async ({ baseCommentOverrides = {}, extraComments = [], prop
182182
resolveComment: vi.fn(),
183183
},
184184
},
185+
focus: vi.fn(),
185186
emit: vi.fn(),
186187
};
187188

@@ -282,15 +283,19 @@ describe('CommentDialog.vue', () => {
282283

283284
const header = wrapper.findComponent(CommentHeaderStub);
284285
header.vm.$emit('resolve');
286+
await nextTick();
285287
expect(superdocStub.activeEditor.commands.acceptTrackedChangeById).toHaveBeenCalledWith(baseComment.commentId);
286288
expect(baseComment.resolveComment).toHaveBeenCalledWith({
287289
email: superdocStoreStub.user.email,
288290
name: superdocStoreStub.user.name,
289291
superdoc: expect.any(Object),
290292
});
293+
expect(superdocStub.focus).toHaveBeenCalledTimes(1);
291294

292295
header.vm.$emit('reject');
296+
await nextTick();
293297
expect(superdocStub.activeEditor.commands.rejectTrackedChangeById).toHaveBeenCalledWith(baseComment.commentId);
298+
expect(superdocStub.focus).toHaveBeenCalledTimes(2);
294299
});
295300

296301
it('calls custom accept handler instead of default behavior when configured', async () => {

packages/superdoc/src/components/CommentsLayer/CommentDialog.vue

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ const handleReject = () => {
400400
commentsStore.lastUpdate = new Date();
401401
activeComment.value = null;
402402
commentsStore.setActiveComment(proxy.$superdoc, activeComment.value);
403+
proxy.$superdoc.focus?.();
403404
});
404405
};
405406
@@ -427,6 +428,7 @@ const handleResolve = () => {
427428
commentsStore.lastUpdate = new Date();
428429
activeComment.value = null;
429430
commentsStore.setActiveComment(proxy.$superdoc, activeComment.value);
431+
proxy.$superdoc.focus?.();
430432
});
431433
};
432434

packages/superdoc/src/stores/comments-store.js

Lines changed: 81 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export const useCommentsStore = defineStore('comments', () => {
2929

3030
const isDebugging = false;
3131
const debounceTimers = {};
32+
const trackedChangeResolutionSnapshots = new WeakMap();
3233

3334
const COMMENT_EVENTS = comments_module_events;
3435
const hasInitializedComments = ref(false);
@@ -188,12 +189,26 @@ export const useCommentsStore = defineStore('comments', () => {
188189

189190
const clearResolvedMetadata = (comment) => {
190191
if (!comment) return;
192+
if (
193+
comment.resolvedTime !== undefined ||
194+
comment.resolvedByEmail !== undefined ||
195+
comment.resolvedByName !== undefined
196+
) {
197+
trackedChangeResolutionSnapshots.set(comment, {
198+
resolvedTime: comment.resolvedTime ?? null,
199+
resolvedByEmail: comment.resolvedByEmail ?? null,
200+
resolvedByName: comment.resolvedByName ?? null,
201+
});
202+
}
191203
// Sets the resolved state to null so it can be restored in the comments sidebar
192204
comment.resolvedTime = null;
193205
comment.resolvedByEmail = null;
194206
comment.resolvedByName = null;
195207
};
196208

209+
const getCommentEventPayload = (comment) =>
210+
typeof comment?.getValues === 'function' ? comment.getValues() : { ...comment };
211+
197212
/**
198213
* Check if a comment originated from the super-editor (or has no explicit source).
199214
* Comments without a source are assumed to be editor-backed for backward compatibility.
@@ -512,6 +527,10 @@ export const useCommentsStore = defineStore('comments', () => {
512527
if (event === 'add') {
513528
const existing = findTrackedChangeById();
514529
if (existing) {
530+
// Undo/redo after accept/reject can rematerialize a previously resolved
531+
// tracked change. Reopen the thread so the bubble is actionable again.
532+
if (existing.resolvedTime) clearResolvedMetadata(existing);
533+
515534
// Already exists (e.g. created during batch import) — update instead of duplicating
516535
// Partial resolution can turn a replacement into insert-only/delete-only, so
517536
// clear fields explicitly when the updated payload no longer includes them.
@@ -533,6 +552,7 @@ export const useCommentsStore = defineStore('comments', () => {
533552
// If we have an update event, simply update the composable comment
534553
const existingTrackedChange = findTrackedChangeById();
535554
if (!existingTrackedChange) return;
555+
if (existingTrackedChange.resolvedTime) clearResolvedMetadata(existingTrackedChange);
536556

537557
// Partial resolution can turn a replacement into insert-only/delete-only, so
538558
// clear fields explicitly when the updated payload no longer includes them.
@@ -960,21 +980,31 @@ export const useCommentsStore = defineStore('comments', () => {
960980
}, 0);
961981
};
962982

963-
const createCommentForTrackChanges = (editor, superdoc, trackedChangesOverride = null) => {
983+
const createCommentForTrackChanges = (editor, superdoc, trackedChangesOverride = null, options = {}) => {
984+
const { reopenResolved = false } = options;
964985
const trackedChanges = trackedChangesOverride ?? trackChangesHelpers.getTrackChanges(editor.state);
965986
const groupedChanges = groupChanges(trackedChanges);
966987
const activeDocumentId = editor?.options?.documentId != null ? String(editor.options.documentId) : null;
967988
if (!activeDocumentId) return;
968989

969-
// Build a Set of existing tracked-change IDs for O(1) lookup.
990+
// Build a Set of existing unresolved tracked-change IDs for O(1) lookup.
970991
// Include both runtime and imported IDs to avoid duplicate threads when
971992
// replay/import flows remap commentId but marks still reference importedId.
972-
const existingIds = new Set();
993+
// History replay can opt in to excluding resolved tracked-change threads so
994+
// undo/redo reopens them when their marks reappear. Initial import rebuilds
995+
// keep resolved IDs in the set so resolved DOCX threads do not reopen on load.
996+
const skipIds = new Set();
973997
commentsList.value.forEach((comment) => {
974998
if (!comment?.trackedChange) return;
975999
if (!belongsToDocument(comment, activeDocumentId)) return;
976-
if (comment.commentId != null) existingIds.add(String(comment.commentId));
977-
if (comment.importedId != null) existingIds.add(String(comment.importedId));
1000+
if (comment.resolvedTime && !reopenResolved) {
1001+
if (comment.commentId != null) skipIds.add(String(comment.commentId));
1002+
if (comment.importedId != null) skipIds.add(String(comment.importedId));
1003+
return;
1004+
}
1005+
if (comment.resolvedTime) return;
1006+
if (comment.commentId != null) skipIds.add(String(comment.commentId));
1007+
if (comment.importedId != null) skipIds.add(String(comment.importedId));
9781008
});
9791009

9801010
// Build a Map of change ID → tracked change entries for O(1) lookup per group.
@@ -991,7 +1021,7 @@ export const useCommentsStore = defineStore('comments', () => {
9911021
// Build comment params directly from grouped changes — no PM dispatch needed
9921022
groupedChanges.forEach(({ insertedMark, deletionMark, formatMark }) => {
9931023
const id = insertedMark?.mark.attrs.id || deletionMark?.mark.attrs.id || formatMark?.mark.attrs.id;
994-
if (!id || existingIds.has(id)) return;
1024+
if (!id || skipIds.has(id)) return;
9951025

9961026
const marks = {
9971027
...(insertedMark && { insertedMark: insertedMark.mark }),
@@ -1012,9 +1042,9 @@ export const useCommentsStore = defineStore('comments', () => {
10121042

10131043
if (params) {
10141044
handleTrackedChangeUpdate({ superdoc, params });
1015-
existingIds.add(String(id));
1016-
if (params.changeId != null) existingIds.add(String(params.changeId));
1017-
if (params.importedId != null) existingIds.add(String(params.importedId));
1045+
skipIds.add(String(id));
1046+
if (params.changeId != null) skipIds.add(String(params.changeId));
1047+
if (params.importedId != null) skipIds.add(String(params.importedId));
10181048
}
10191049
});
10201050

@@ -1062,10 +1092,11 @@ export const useCommentsStore = defineStore('comments', () => {
10621092
* @param {string | null} activeDocumentId Document currently being synced.
10631093
* @returns {void}
10641094
*/
1065-
const pruneStaleTrackedChangeComments = (liveTrackedChangeIds, activeDocumentId) => {
1095+
const pruneStaleTrackedChangeComments = (liveTrackedChangeIds, activeDocumentId, superdoc = null) => {
10661096
if (!(liveTrackedChangeIds instanceof Set) || !activeDocumentId) return;
10671097

10681098
const removedIds = new Set();
1099+
const restoredComments = [];
10691100
const previousComments = [...commentsList.value];
10701101

10711102
commentsList.value = commentsList.value.filter((comment) => {
@@ -1078,12 +1109,32 @@ export const useCommentsStore = defineStore('comments', () => {
10781109
const hasLiveImportedId = Boolean(importedId && liveTrackedChangeIds.has(importedId));
10791110

10801111
if ((!commentId && !importedId) || hasLiveCommentId || hasLiveImportedId) return true;
1112+
if (comment.resolvedTime) return true;
1113+
1114+
const resolutionSnapshot = trackedChangeResolutionSnapshots.get(comment);
1115+
if (resolutionSnapshot) {
1116+
comment.resolvedTime = resolutionSnapshot.resolvedTime ?? Date.now();
1117+
comment.resolvedByEmail = resolutionSnapshot.resolvedByEmail ?? null;
1118+
comment.resolvedByName = resolutionSnapshot.resolvedByName ?? null;
1119+
restoredComments.push(comment);
1120+
return true;
1121+
}
10811122

10821123
if (commentId) removedIds.add(commentId);
10831124
if (importedId) removedIds.add(importedId);
10841125
return false;
10851126
});
10861127

1128+
restoredComments.forEach((comment) => {
1129+
const payload = getCommentEventPayload(comment);
1130+
const event = {
1131+
type: COMMENT_EVENTS.UPDATE,
1132+
comment: payload,
1133+
};
1134+
syncCommentsToClients(superdoc, event);
1135+
superdoc?.emit?.('comments-update', event);
1136+
});
1137+
10871138
if (!removedIds.size) return;
10881139

10891140
let didRemoveDescendants = true;
@@ -1110,6 +1161,24 @@ export const useCommentsStore = defineStore('comments', () => {
11101161
});
11111162
}
11121163

1164+
const removedComments = previousComments.filter((comment) => {
1165+
if (!belongsToDocument(comment, activeDocumentId)) return false;
1166+
const commentId = comment.commentId != null ? String(comment.commentId) : null;
1167+
const importedId = comment.importedId != null ? String(comment.importedId) : null;
1168+
return (commentId && removedIds.has(commentId)) || (importedId && removedIds.has(importedId));
1169+
});
1170+
1171+
removedComments.forEach((comment) => {
1172+
const payload = getCommentEventPayload(comment);
1173+
const event = {
1174+
type: COMMENT_EVENTS.DELETED,
1175+
comment: payload,
1176+
changes: [{ key: 'deleted', commentId: payload.commentId, fileId: payload.fileId }],
1177+
};
1178+
syncCommentsToClients(superdoc, event);
1179+
superdoc?.emit?.('comments-update', event);
1180+
});
1181+
11131182
const activeCommentId = activeComment.value != null ? String(activeComment.value) : null;
11141183
const activeCommentBelongsToActiveDocument = previousComments.some((comment) => {
11151184
const commentId = comment.commentId != null ? String(comment.commentId) : null;
@@ -1148,8 +1217,8 @@ export const useCommentsStore = defineStore('comments', () => {
11481217
liveTrackedChangeIds.add(String(id));
11491218
});
11501219

1151-
pruneStaleTrackedChangeComments(liveTrackedChangeIds, activeDocumentId);
1152-
createCommentForTrackChanges(editor, superdoc, trackedChanges);
1220+
pruneStaleTrackedChangeComments(liveTrackedChangeIds, activeDocumentId, superdoc);
1221+
createCommentForTrackChanges(editor, superdoc, trackedChanges, { reopenResolved: true });
11531222
};
11541223

11551224
const normalizeDocxSchemaForExport = (value) => {

0 commit comments

Comments
 (0)