Skip to content

Commit a429e7b

Browse files
authored
fix(comments): track change edits in body get matched in comments bubble (#3305)
* feat(comments): implement refresh functionality for tracked-change comments by IDs * chore: add comments * fix: shallow equal build * fix(comments): inline match logic * refactor: use exported equal check * fix: use correct indexing for doc position * test: use actual mapping
1 parent 51a8615 commit a429e7b

7 files changed

Lines changed: 551 additions & 44 deletions

File tree

packages/superdoc/src/SuperDoc.test.js

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { h, defineComponent, ref, shallowRef, reactive, nextTick } from 'vue';
44
import { DOCX } from '@superdoc/common';
55
import { Schema } from 'prosemirror-model';
66
import { EditorState, TextSelection } from 'prosemirror-state';
7+
import { Mapping, StepMap } from 'prosemirror-transform';
78
import { ySyncPluginKey } from 'y-prosemirror';
89
import { Extension } from '../../super-editor/src/editors/v1/core/Extension.js';
910
import {
@@ -120,6 +121,7 @@ vi.mock('@superdoc/super-editor', () => ({
120121
SuperEditor: SuperEditorStub,
121122
AIWriter: AIWriterStub,
122123
getTrackedChangeIndex: getTrackedChangeIndexMock,
124+
TrackChangesBasePluginKey: 'TrackChangesBasePluginKey',
123125
PresentationEditor: class PresentationEditorMock {
124126
static getInstance(documentId) {
125127
return mockState.instances.get(documentId);
@@ -194,6 +196,7 @@ const buildCommentsStore = () => ({
194196
handleEditorLocationsUpdate: vi.fn(),
195197
clearEditorCommentPositions: vi.fn(),
196198
handleTrackedChangeUpdate: vi.fn(),
199+
refreshTrackedChangeCommentsByIds: vi.fn(),
197200
syncTrackedChangePositionsWithDocument: vi.fn(),
198201
syncTrackedChangeComments: vi.fn(),
199202
removePendingComment: vi.fn(),
@@ -1001,6 +1004,72 @@ describe('SuperDoc.vue', () => {
10011004
expect(commentsStoreStub.syncTrackedChangeComments).not.toHaveBeenCalled();
10021005
});
10031006

1007+
it('refreshes only touched tracked-change comments from normal body transactions', async () => {
1008+
const superdocStub = createSuperdocStub();
1009+
const wrapper = await mountComponent(superdocStub);
1010+
await nextTick();
1011+
1012+
const options = wrapper.findComponent(SuperEditorStub).props('options');
1013+
const editorMock = { options: { documentId: 'doc-1' } };
1014+
const transaction = {
1015+
getMeta: vi.fn((key) =>
1016+
key === 'TrackChangesBasePluginKey'
1017+
? {
1018+
insertedMark: { attrs: { id: 'tracked-insert-1' } },
1019+
deletionMark: { attrs: { id: 'tracked-delete-1' } },
1020+
formatMark: { attrs: { id: 'tracked-format-1' } },
1021+
}
1022+
: undefined,
1023+
),
1024+
};
1025+
1026+
options.onTransaction({ editor: editorMock, transaction, duration: 3 });
1027+
1028+
expect(commentsStoreStub.syncTrackedChangePositionsWithDocument).not.toHaveBeenCalled();
1029+
expect(commentsStoreStub.syncTrackedChangeComments).not.toHaveBeenCalled();
1030+
expect(commentsStoreStub.refreshTrackedChangeCommentsByIds).not.toHaveBeenCalled();
1031+
1032+
await Promise.resolve();
1033+
expect(commentsStoreStub.refreshTrackedChangeCommentsByIds).toHaveBeenCalledWith({
1034+
superdoc: superdocStub,
1035+
editor: editorMock,
1036+
changeIds: ['tracked-insert-1', 'tracked-delete-1', 'tracked-format-1'],
1037+
broadcastChanges: true,
1038+
});
1039+
});
1040+
1041+
it('refreshes tracked-change comments found in changed transaction ranges', async () => {
1042+
const superdocStub = createSuperdocStub();
1043+
const wrapper = await mountComponent(superdocStub);
1044+
await nextTick();
1045+
1046+
const options = wrapper.findComponent(SuperEditorStub).props('options');
1047+
const editorMock = { options: { documentId: 'doc-1' } };
1048+
const trackedNode = {
1049+
marks: [{ type: { name: 'trackInsert' }, attrs: { id: 'tracked-range-1' } }],
1050+
};
1051+
const transaction = {
1052+
docChanged: true,
1053+
doc: {
1054+
content: { size: 20 },
1055+
nodesBetween: vi.fn((from, to, visitor) => visitor(trackedNode)),
1056+
},
1057+
mapping: new Mapping([new StepMap([4, 0, 1])]),
1058+
getMeta: vi.fn(() => undefined),
1059+
};
1060+
1061+
options.onTransaction({ editor: editorMock, transaction, duration: 3 });
1062+
1063+
await Promise.resolve();
1064+
expect(transaction.doc.nodesBetween).toHaveBeenCalledWith(3, 6, expect.any(Function));
1065+
expect(commentsStoreStub.refreshTrackedChangeCommentsByIds).toHaveBeenCalledWith({
1066+
superdoc: superdocStub,
1067+
editor: editorMock,
1068+
changeIds: ['tracked-range-1'],
1069+
broadcastChanges: true,
1070+
});
1071+
});
1072+
10041073
it('reconciles replay updates by importedId before commentId to avoid duplicate comments', async () => {
10051074
const superdocStub = createSuperdocStub();
10061075
const wrapper = await mountComponent(superdocStub);

packages/superdoc/src/SuperDoc.vue

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,13 @@ import { useSuperdocStore } from '@superdoc/stores/superdoc-store';
3333
import { useCommentsStore } from '@superdoc/stores/comments-store';
3434

3535
import { DOCX, PDF, HTML } from '@superdoc/common';
36-
import { SuperEditor, AIWriter, PresentationEditor, getTrackedChangeIndex } from '@superdoc/super-editor';
36+
import {
37+
SuperEditor,
38+
AIWriter,
39+
PresentationEditor,
40+
getTrackedChangeIndex,
41+
TrackChangesBasePluginKey,
42+
} from '@superdoc/super-editor';
3743
import { ySyncPluginKey } from 'y-prosemirror';
3844
import HtmlViewer from './components/HtmlViewer/HtmlViewer.vue';
3945
import useComment from './components/CommentsLayer/use-comment';
@@ -45,6 +51,7 @@ import { getVisibleThreadAnchorClientY } from './helpers/comment-focus.js';
4551
import { useUiFontFamily } from './composables/useUiFontFamily.js';
4652
import { usePasswordPrompt } from './composables/use-password-prompt.js';
4753
import { useFindReplace } from './composables/use-find-replace.js';
54+
import { collectTouchedTrackedChangeIds } from './helpers/collect-touched-tracked-change-ids.js';
4855
import SurfaceHost from './components/surfaces/SurfaceHost.vue';
4956

5057
const PdfViewer = defineAsyncComponent(() => import('./components/PdfViewer/PdfViewer.vue'));
@@ -116,6 +123,7 @@ const {
116123
showAddComment,
117124
handleEditorLocationsUpdate,
118125
handleTrackedChangeUpdate,
126+
refreshTrackedChangeCommentsByIds,
119127
syncTrackedChangePositionsWithDocument,
120128
syncTrackedChangeComments,
121129
addComment,
@@ -258,18 +266,31 @@ const flushQueuedTrackedChangeCommentResync = () => {
258266
queuedTrackedChangeCommentResync = null;
259267
if (!pendingResync?.editor) return;
260268

261-
syncTrackedChangeComments({
269+
if (pendingResync.fullResync) {
270+
syncTrackedChangeComments({
271+
superdoc: proxy.$superdoc,
272+
editor: pendingResync.editor,
273+
broadcastChanges: pendingResync.broadcastChanges,
274+
});
275+
return;
276+
}
277+
278+
refreshTrackedChangeCommentsByIds({
262279
superdoc: proxy.$superdoc,
263280
editor: pendingResync.editor,
281+
changeIds: Array.from(pendingResync.changeIds ?? []),
264282
broadcastChanges: pendingResync.broadcastChanges,
265283
});
266284
};
267285

268-
const queueTrackedChangeCommentResync = ({ editor, broadcastChanges = true } = {}) => {
269-
if (!editor) return;
286+
const queueTrackedChangeCommentResync = ({ editor, changeIds = null, broadcastChanges = true } = {}) => {
287+
if (!editor || (changeIds && !changeIds.size)) return;
270288

289+
const existingChangeIds = queuedTrackedChangeCommentResync?.changeIds ?? new Set();
271290
queuedTrackedChangeCommentResync = {
272291
editor,
292+
fullResync: !changeIds || Boolean(queuedTrackedChangeCommentResync?.fullResync),
293+
changeIds: changeIds ? new Set([...existingChangeIds, ...changeIds]) : existingChangeIds,
273294
broadcastChanges: Boolean(queuedTrackedChangeCommentResync?.broadcastChanges) || Boolean(broadcastChanges),
274295
};
275296

@@ -1159,6 +1180,10 @@ const shouldResyncTrackedChangeThreads = (transaction, ySyncMeta = transaction?.
11591180
return isLocalHistoryUndoRedo || isLocalCollabUndoRedo || isCollaborationReplayTransaction(transaction, ySyncMeta);
11601181
};
11611182

1183+
const collectTouchedChangeIds = (transaction) => {
1184+
return collectTouchedTrackedChangeIds(transaction, { trackChangesPluginKey: TrackChangesBasePluginKey });
1185+
};
1186+
11621187
const onEditorTransaction = (payload = {}) => {
11631188
const { editor, transaction } = payload;
11641189
const ySyncMeta = transaction?.getMeta?.(ySyncPluginKey);
@@ -1174,6 +1199,11 @@ const onEditorTransaction = (payload = {}) => {
11741199
// collaboration comment update is already shared through the comments ydoc.
11751200
broadcastChanges: !isPeerCollaborationReplayTransaction(transaction, ySyncMeta),
11761201
});
1202+
} else {
1203+
queueTrackedChangeCommentResync({
1204+
editor,
1205+
changeIds: collectTouchedChangeIds(transaction),
1206+
});
11771207
}
11781208

11791209
emitEditorTransaction(buildEditorTransactionPayload(payload));
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/**
2+
* Collect tracked-change mark IDs from ranges touched by a ProseMirror transaction.
3+
*
4+
* @param {import('prosemirror-state').Transaction} transaction
5+
* @param {object} [options]
6+
* @param {object|null} [options.trackChangesPluginKey] - The TrackChangesBasePluginKey to read metadata from
7+
* @returns {Set<string>} Set of tracked-change mark IDs
8+
*/
9+
export function collectTouchedTrackedChangeIds(transaction, { trackChangesPluginKey = null } = {}) {
10+
const ids = new Set();
11+
const addMarkId = (mark) => {
12+
const id = mark?.attrs?.id;
13+
if (id != null) ids.add(String(id));
14+
};
15+
16+
// AIDEV-NOTE: Existing tracked-change edits can update the live mark text
17+
// without reporting that mark in TrackChangesBasePluginKey metadata. Keep
18+
// the changed-range scan so the sidebar bubble refreshes for those edits.
19+
const meta = trackChangesPluginKey ? transaction?.getMeta?.(trackChangesPluginKey) : null;
20+
if (meta) {
21+
[meta.insertedMark, meta.deletionMark, meta.formatMark].forEach(addMarkId);
22+
}
23+
24+
if (!transaction?.docChanged || !transaction?.doc || !transaction?.mapping?.maps?.length) return ids;
25+
26+
// Map each step's output range through all subsequent steps to get coordinates
27+
// valid in transaction.doc (the final document). Individual step maps report
28+
// newStart/newEnd relative to intermediate documents, not the final doc —
29+
// using them directly causes wrong lookups in multi-step transactions
30+
// (e.g. IME/composition, paste, batched edits).
31+
const docSize = transaction.doc.content.size;
32+
transaction.mapping.maps.forEach((stepMap, stepIndex) => {
33+
stepMap.forEach((oldStart, oldEnd, newStart, newEnd) => {
34+
const mappingOffset = stepIndex + 1;
35+
const mappedFrom = transaction.mapping.slice(mappingOffset).map(newStart, 1);
36+
const mappedTo = transaction.mapping.slice(mappingOffset).map(newEnd, -1);
37+
const from = Math.max(0, mappedFrom - 1);
38+
const to = Math.min(docSize, mappedTo + 1);
39+
if (from >= to) return;
40+
41+
transaction.doc.nodesBetween(from, to, (node) => {
42+
node.marks?.forEach((mark) => {
43+
const markName = mark.type?.name;
44+
if (markName === 'trackInsert' || markName === 'trackDelete' || markName === 'trackFormat') addMarkId(mark);
45+
});
46+
});
47+
});
48+
});
49+
50+
return ids;
51+
}

0 commit comments

Comments
 (0)