Skip to content

Commit 94f0056

Browse files
authored
fix(tracked-changes): sync tracked changes store on undo and redo (#2164)
* feat(comments): synchronize tracked change positions with document on undo/redo actions * refactor: match editorCommentPositions for undo * fix: add edge cases for commentId vs importedId * fix: use the same logic for keys in sidebar * fix: further normalize comment ids * chore: cleanup tests and dupe function
1 parent 240fb66 commit 94f0056

8 files changed

Lines changed: 596 additions & 34 deletions

File tree

packages/super-editor/src/core/presentation-editor/PresentationEditor.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2644,20 +2644,17 @@ export class PresentationEditor extends EventEmitter {
26442644
this.#epochMapper.recordTransaction(transaction);
26452645
this.#selectionSync.setDocEpoch(this.#epochMapper.getCurrentEpoch());
26462646

2647+
const inputType = transaction.getMeta?.('inputType');
26472648
// Detect Y.js-origin transactions (remote collaboration changes).
26482649
// These bypass the blockNodePlugin's sdBlockRev increment to prevent
26492650
// feedback loops, so the FlowBlockCache's fast revision comparison
2650-
// cannot be trusted — signal it to fall through to JSON comparison.
2651+
// cannot be trusted. History undo/redo can also restore tracked-mark-only
2652+
// changes where visible text stays the same, so use the same JSON fallback.
26512653
const ySyncMeta = transaction.getMeta?.(ySyncPluginKey);
2652-
if (ySyncMeta?.isChangeOrigin && transaction.docChanged) {
2653-
this.#flowBlockCache?.setHasExternalChanges(true);
2654-
}
2655-
// History undo/redo can restore prior paragraph content while preserving/reusing
2656-
// sdBlockRev values, which makes the cache's fast revision check unsafe.
2657-
// Force JSON comparison for this render cycle to avoid stale paragraph reuse.
2658-
const inputType = transaction.getMeta?.('inputType');
2659-
const isHistoryType = inputType === 'historyUndo' || inputType === 'historyRedo';
2660-
if (isHistoryType && transaction.docChanged) {
2654+
const shouldBypassFastRevision =
2655+
transaction.docChanged &&
2656+
(ySyncMeta?.isChangeOrigin || inputType === 'historyUndo' || inputType === 'historyRedo');
2657+
if (shouldBypassFastRevision) {
26612658
this.#flowBlockCache?.setHasExternalChanges(true);
26622659
}
26632660
}

packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ const {
2525
mockOnHeaderFooterDataUpdate,
2626
mockUpdateYdocDocxData,
2727
mockEditorOverlayManager,
28+
mockFlowBlockCacheInstances,
29+
MockFlowBlockCache,
2830
} = vi.hoisted(() => {
2931
const createDefaultConverter = () => ({
3032
headers: {
@@ -106,6 +108,19 @@ const {
106108
};
107109

108110
const editors: Array<{ editor: ReturnType<typeof createSectionEditor> }> = [];
111+
const mockFlowBlockCacheInstances: Array<{
112+
clear: ReturnType<typeof vi.fn>;
113+
setHasExternalChanges: ReturnType<typeof vi.fn>;
114+
}> = [];
115+
116+
class MockFlowBlockCache {
117+
clear = vi.fn();
118+
setHasExternalChanges = vi.fn();
119+
120+
constructor() {
121+
mockFlowBlockCacheInstances.push(this);
122+
}
123+
}
109124

110125
return {
111126
createDefaultConverter,
@@ -145,6 +160,8 @@ const {
145160
getActiveEditorHost: vi.fn(() => null),
146161
destroy: vi.fn(),
147162
})),
163+
mockFlowBlockCacheInstances,
164+
MockFlowBlockCache,
148165
};
149166
});
150167

@@ -219,6 +236,7 @@ vi.mock('@superdoc/pm-adapter', async (importOriginal) => {
219236
return {
220237
...actual,
221238
toFlowBlocks: mockToFlowBlocks,
239+
FlowBlockCache: MockFlowBlockCache,
222240
};
223241
});
224242

@@ -319,6 +337,7 @@ describe('PresentationEditor', () => {
319337
};
320338
mockEditorConverterStore.mediaFiles = {};
321339
createdSectionEditors.length = 0;
340+
mockFlowBlockCacheInstances.length = 0;
322341

323342
// Reset static instances
324343
(PresentationEditor as typeof PresentationEditor & { instances: Map<string, unknown> }).instances = new Map();
@@ -2404,6 +2423,48 @@ describe('PresentationEditor', () => {
24042423
expect(layoutUpdatedCount).toBeGreaterThan(afterDocUpdate);
24052424
});
24062425

2426+
it('marks the flow-block cache dirty for history undo and redo updates', async () => {
2427+
mockIncrementalLayout.mockResolvedValue(buildLayoutResult());
2428+
2429+
editor = new PresentationEditor({
2430+
element: container,
2431+
documentId: 'test-doc',
2432+
});
2433+
2434+
const mockEditorInstance = (Editor as unknown as MockedEditor).mock.results[
2435+
(Editor as unknown as MockedEditor).mock.results.length - 1
2436+
].value;
2437+
2438+
await waitForLayoutUpdate();
2439+
2440+
const flowBlockCache = mockFlowBlockCacheInstances.at(-1);
2441+
expect(flowBlockCache).toBeDefined();
2442+
flowBlockCache!.setHasExternalChanges.mockClear();
2443+
2444+
const onCalls = mockEditorInstance.on as unknown as Mock;
2445+
const updateCall = onCalls.mock.calls.find((call) => call[0] === 'update');
2446+
expect(updateCall).toBeDefined();
2447+
2448+
const handleUpdate = updateCall![1] as (payload: { transaction: { docChanged: boolean; getMeta: Mock } }) => void;
2449+
const makeTransaction = (inputType: string) => ({
2450+
docChanged: true,
2451+
getMeta: vi.fn((key: string) => (key === 'inputType' ? inputType : undefined)),
2452+
mapping: {
2453+
appendMapping: vi.fn(),
2454+
slice: vi.fn(() => ({
2455+
appendMapping: vi.fn(),
2456+
})),
2457+
},
2458+
});
2459+
2460+
handleUpdate({ transaction: makeTransaction('historyUndo') });
2461+
handleUpdate({ transaction: makeTransaction('historyRedo') });
2462+
2463+
expect(flowBlockCache!.setHasExternalChanges).toHaveBeenCalledTimes(2);
2464+
expect(flowBlockCache!.setHasExternalChanges).toHaveBeenNthCalledWith(1, true);
2465+
expect(flowBlockCache!.setHasExternalChanges).toHaveBeenNthCalledWith(2, true);
2466+
});
2467+
24072468
it('should remove pageStyleUpdate listener on destroy', () => {
24082469
editor = new PresentationEditor({
24092470
element: container,

packages/superdoc/src/SuperDoc.vue

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ const {
9090
showAddComment,
9191
handleEditorLocationsUpdate,
9292
handleTrackedChangeUpdate,
93+
syncTrackedChangePositionsWithDocument,
9394
addComment,
9495
getComment,
9596
COMMENT_EVENTS,
@@ -686,6 +687,15 @@ const onEditorCommentsUpdate = (params = {}) => {
686687
};
687688
688689
const onEditorTransaction = ({ editor, transaction, duration }) => {
690+
const inputType = transaction?.getMeta?.('inputType');
691+
692+
// Call sync on editor transaction but only if it's undo or redo
693+
// This could be extended to other listeners in the future
694+
if (inputType === 'historyUndo' || inputType === 'historyRedo') {
695+
const documentId = editor?.options?.documentId;
696+
syncTrackedChangePositionsWithDocument({ documentId, editor });
697+
}
698+
689699
if (typeof proxy.$superdoc.config.onTransaction === 'function') {
690700
proxy.$superdoc.config.onTransaction({ editor, transaction, duration });
691701
}

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ const props = defineProps({
6666
6767
const superdocStore = useSuperdocStore();
6868
const commentsStore = useCommentsStore();
69+
const { getCommentAliasIds, getCommentPositionKey, resolveCommentPositionEntry } = commentsStore;
6970
7071
const { getFloatingComments, activeComment, editorCommentPositions, pendingComment } = storeToRefs(commentsStore);
7172
const { activeZoom } = storeToRefs(superdocStore);
@@ -76,10 +77,15 @@ const commentsRenderKey = ref(0);
7677
// Resolve activeComment (which stores commentId) to the position key used by allPositions
7778
// (which prefers importedId). Without this, imported Word comments where importedId !== commentId
7879
// would fail the template guard and could unmount when scrolled out of the observer viewport.
80+
const resolveLayoutKey = (commentOrId, preferredId) => {
81+
const { key } = resolveCommentPositionEntry(commentOrId, preferredId);
82+
if (key) return key;
83+
return getCommentAliasIds(commentOrId)[0] ?? getCommentPositionKey(commentOrId);
84+
};
85+
7986
const activeCommentKey = computed(() => {
8087
if (!activeComment.value) return null;
81-
const comment = commentsStore.getComment(activeComment.value);
82-
return comment ? commentsStore.getCommentPositionKey(comment) : null;
88+
return resolveLayoutKey(activeComment.value);
8389
});
8490
8591
// Heights: measured (actual) or estimated. Seeded from module-level cache to
@@ -98,8 +104,7 @@ const observedElements = new Set();
98104
99105
// Compute anchor position for a comment from editor position data
100106
const getAnchorTop = (comment) => {
101-
const key = commentsStore.getCommentPositionKey(comment);
102-
const positionEntry = editorCommentPositions.value[key];
107+
const { entry: positionEntry } = resolveCommentPositionEntry(comment);
103108
104109
if (props.currentDocument.type === 'application/pdf') {
105110
const zoom = (activeZoom.value ?? 100) / 100;
@@ -131,7 +136,7 @@ const allPositions = computed(() => {
131136
132137
const positions = [];
133138
for (const comment of comments) {
134-
const key = commentsStore.getCommentPositionKey(comment);
139+
const key = resolveLayoutKey(comment);
135140
const top = getAnchorTop(comment);
136141
if (!key || typeof top !== 'number' || isNaN(top)) continue;
137142
@@ -246,14 +251,14 @@ const handleDialog = (dialog) => {
246251
nextTick(() => {
247252
const bounds = elementRef.value?.getBoundingClientRect();
248253
if (!bounds || bounds.height <= 0) return;
249-
const key = commentsStore.getCommentPositionKey(rawId);
254+
const key = resolveLayoutKey(rawId, rawId);
250255
if (key) storeHeight(key, bounds.height);
251256
});
252257
};
253258
254259
// Re-measure a specific comment dialog when it signals a resize (e.g. text truncation toggle)
255260
const handleResize = (comment) => {
256-
const key = commentsStore.getCommentPositionKey(comment);
261+
const key = resolveLayoutKey(comment);
257262
if (!key) return;
258263
nextTick(() => {
259264
const el = placeholderRefs.value[key];
@@ -319,7 +324,7 @@ watch(activeComment, () => {
319324
if (!activeComment.value) return;
320325
const comment = commentsStore.getComment(activeComment.value);
321326
if (!comment) return;
322-
const key = commentsStore.getCommentPositionKey(comment);
327+
const key = resolveLayoutKey(comment);
323328
if (!key) return;
324329
325330
nextTick(() => {

0 commit comments

Comments
 (0)