Skip to content

Commit 988598d

Browse files
palmer-clpalmer-cl
authored
feat(tracked-changes): allow partial tracked change resolution (#2252)
* fix(track-changes): allow for partial tracked-change accept and reject * refactor(track-changes): adjust context menu actions for accepting and rejecting tracked changes * fix(context-menu): enhance tracked changes handling for right-click actions within expanded selections * fix: resolve id for doc or cache * refactor: extract shared helper * test: more extractions for helpers in tests * chore: add clarifying comment
1 parent 128a1fb commit 988598d

17 files changed

Lines changed: 1454 additions & 249 deletions

‎packages/super-editor/src/components/context-menu/menuItems.js‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,11 @@ export function getItems(context, customItems = [], includeDefaultItems = true)
150150
label: TEXTS.trackChangesAccept,
151151
isDefault: true,
152152
action: (editor, context) => {
153-
if (context?.trackedChangeId) {
154-
editor.commands.acceptTrackedChangeById(context.trackedChangeId);
155-
} else {
156-
editor.commands.acceptTrackedChangeBySelection();
157-
}
153+
editor.commands.acceptTrackedChangeFromContextMenu({
154+
from: context?.selectionStart,
155+
to: context?.selectionEnd,
156+
trackedChangeId: context?.trackedChangeId,
157+
});
158158
},
159159
showWhen: (context) => {
160160
const { trigger, isTrackedChange } = context;
@@ -167,11 +167,11 @@ export function getItems(context, customItems = [], includeDefaultItems = true)
167167
icon: ICONS.trackChangesReject,
168168
isDefault: true,
169169
action: (editor, context) => {
170-
if (context?.trackedChangeId) {
171-
editor.commands.rejectTrackedChangeById(context.trackedChangeId);
172-
} else {
173-
editor.commands.rejectTrackedChangeOnSelection();
174-
}
170+
editor.commands.rejectTrackedChangeFromContextMenu({
171+
from: context?.selectionStart,
172+
to: context?.selectionEnd,
173+
trackedChangeId: context?.trackedChangeId,
174+
});
175175
},
176176
showWhen: (context) => {
177177
const { trigger, isTrackedChange } = context;

‎packages/super-editor/src/components/context-menu/tests/menuItems.test.js‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,88 @@ describe('menuItems.js', () => {
126126
expect(itemIds).not.toContain('track-changes-reject');
127127
});
128128

129+
it('routes tracked-change context-menu actions through selection commands when text is selected', () => {
130+
const acceptTrackedChangeFromContextMenu = vi.fn();
131+
const rejectTrackedChangeFromContextMenu = vi.fn();
132+
133+
mockEditor.commands = {
134+
acceptTrackedChangeFromContextMenu,
135+
rejectTrackedChangeFromContextMenu,
136+
};
137+
138+
mockContext = createMockContext({
139+
editor: mockEditor,
140+
trigger: TRIGGERS.click,
141+
hasSelection: true,
142+
isTrackedChange: true,
143+
selectionStart: 10,
144+
selectionEnd: 14,
145+
trackedChangeId: 'tracked-change-1',
146+
});
147+
148+
const sections = getItems(mockContext);
149+
const trackSection = sections.find((section) => section.id === 'track-changes');
150+
const acceptItem = trackSection?.items.find((item) => item.id === 'track-changes-accept');
151+
const rejectItem = trackSection?.items.find((item) => item.id === 'track-changes-reject');
152+
153+
expect(acceptItem).toBeDefined();
154+
expect(rejectItem).toBeDefined();
155+
156+
acceptItem.action(mockEditor, mockContext);
157+
rejectItem.action(mockEditor, mockContext);
158+
159+
expect(acceptTrackedChangeFromContextMenu).toHaveBeenCalledWith({
160+
from: 10,
161+
to: 14,
162+
trackedChangeId: 'tracked-change-1',
163+
});
164+
expect(rejectTrackedChangeFromContextMenu).toHaveBeenCalledWith({
165+
from: 10,
166+
to: 14,
167+
trackedChangeId: 'tracked-change-1',
168+
});
169+
});
170+
171+
it('routes tracked-change context-menu actions through toolbar commands for collapsed selections', () => {
172+
const acceptTrackedChangeFromContextMenu = vi.fn();
173+
const rejectTrackedChangeFromContextMenu = vi.fn();
174+
175+
mockEditor.commands = {
176+
acceptTrackedChangeFromContextMenu,
177+
rejectTrackedChangeFromContextMenu,
178+
};
179+
180+
mockContext = createMockContext({
181+
editor: mockEditor,
182+
trigger: TRIGGERS.click,
183+
hasSelection: false,
184+
isTrackedChange: true,
185+
trackedChangeId: 'tracked-change-2',
186+
});
187+
188+
const sections = getItems(mockContext);
189+
const trackSection = sections.find((section) => section.id === 'track-changes');
190+
const acceptItem = trackSection?.items.find((item) => item.id === 'track-changes-accept');
191+
const rejectItem = trackSection?.items.find((item) => item.id === 'track-changes-reject');
192+
193+
expect(acceptItem).toBeDefined();
194+
expect(rejectItem).toBeDefined();
195+
196+
acceptItem.action(mockEditor, mockContext);
197+
rejectItem.action(mockEditor, mockContext);
198+
199+
expect(acceptTrackedChangeFromContextMenu).toHaveBeenCalledWith({
200+
from: 10,
201+
to: 10,
202+
trackedChangeId: 'tracked-change-2',
203+
});
204+
expect(rejectTrackedChangeFromContextMenu).toHaveBeenCalledWith({
205+
from: 10,
206+
to: 10,
207+
trackedChangeId: 'tracked-change-2',
208+
});
209+
});
210+
129211
it('should filter AI items when AI module is not enabled', () => {
130212
const sections = getItems(mockContext);
131213

‎packages/super-editor/src/components/context-menu/tests/utils.test.js‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,19 @@ import { undoDepth, redoDepth } from 'prosemirror-history';
6161
import { yUndoPluginKey } from 'y-prosemirror';
6262
import { isCellSelection as isCellSelectionMock } from '@extensions/table/tableHelpers/isCellSelection.js';
6363
import { selectedRect as selectedRectMock } from 'prosemirror-tables';
64+
import {
65+
collectTrackedChanges,
66+
collectTrackedChangesForContext,
67+
} from '@extensions/track-changes/permission-helpers.js';
6468

6569
// Get the mocked functions
6670
const mockReadFromClipboard = vi.mocked(readFromClipboard);
6771
const mockSelectionHasNodeOrMark = vi.mocked(selectionHasNodeOrMark);
6872
const mockUndoDepth = vi.mocked(undoDepth);
6973
const mockRedoDepth = vi.mocked(redoDepth);
7074
const mockYUndoPluginKeyGetState = vi.mocked(yUndoPluginKey.getState);
75+
const mockCollectTrackedChanges = vi.mocked(collectTrackedChanges);
76+
const mockCollectTrackedChangesForContext = vi.mocked(collectTrackedChangesForContext);
7177

7278
describe('utils.js', () => {
7379
let mockEditor;
@@ -84,6 +90,10 @@ describe('utils.js', () => {
8490
mockUndoDepth.mockReturnValue(1);
8591
mockRedoDepth.mockReturnValue(1);
8692
mockYUndoPluginKeyGetState.mockReturnValue({ undoManager: { undoStack: [1], redoStack: [1] } });
93+
mockCollectTrackedChanges.mockReset();
94+
mockCollectTrackedChangesForContext.mockReset();
95+
mockCollectTrackedChanges.mockReturnValue([]);
96+
mockCollectTrackedChangesForContext.mockReturnValue([]);
8797

8898
// Create editor with default configuration
8999
mockEditor = createMockEditor({
@@ -255,6 +265,57 @@ describe('utils.js', () => {
255265
expect(Array.isArray(context.trackedChanges)).toBe(true);
256266
});
257267

268+
it('prefers preserved selection for right-click context when the live selection collapsed inside it', async () => {
269+
const mockEvent = { clientX: 300, clientY: 400 };
270+
271+
mockSelectionHasNodeOrMark.mockReturnValue(false);
272+
mockEditor.options.preservedSelection = { from: 10, to: 15 };
273+
mockEditor.view.state.selection.empty = true;
274+
mockEditor.view.state.selection.from = 12;
275+
mockEditor.view.state.selection.to = 12;
276+
mockEditor.view.posAtCoords.mockReturnValue({ pos: 12 });
277+
mockEditor.view.state.doc.nodeAt.mockReturnValue({ type: { name: 'text' } });
278+
mockEditor.view.state.doc.textBetween.mockReturnValue('selected text');
279+
mockEditor.view.state.doc.resolve.mockReturnValue({
280+
marks: vi.fn(() => []),
281+
nodeBefore: null,
282+
nodeAfter: null,
283+
});
284+
285+
const context = await getEditorContext(mockEditor, mockEvent);
286+
287+
expect(context.hasSelection).toBe(true);
288+
expect(context.selectionStart).toBe(10);
289+
expect(context.selectionEnd).toBe(15);
290+
expect(context.selectedText).toBe('selected text');
291+
});
292+
293+
it('uses selection-scoped tracked changes for right-click actions inside an expanded selection', async () => {
294+
const mockEvent = { clientX: 300, clientY: 400 };
295+
296+
mockSelectionHasNodeOrMark.mockReturnValue(false);
297+
mockEditor.view.state.selection.empty = false;
298+
mockEditor.view.state.selection.from = 10;
299+
mockEditor.view.state.selection.to = 15;
300+
mockEditor.view.posAtCoords.mockReturnValue({ pos: 12 });
301+
mockEditor.view.state.doc.nodeAt.mockReturnValue({ type: { name: 'text' } });
302+
mockEditor.view.state.doc.textBetween.mockReturnValue('selected text');
303+
mockEditor.view.state.doc.resolve.mockReturnValue({
304+
marks: vi.fn(() => []),
305+
nodeBefore: null,
306+
nodeAfter: null,
307+
});
308+
309+
await getEditorContext(mockEditor, mockEvent);
310+
311+
expect(mockCollectTrackedChanges).toHaveBeenCalledWith({
312+
state: mockEditor.view.state,
313+
from: 10,
314+
to: 15,
315+
});
316+
expect(mockCollectTrackedChangesForContext).not.toHaveBeenCalled();
317+
});
318+
258319
it('should detect tracked change marks directly at the resolved cursor position', async () => {
259320
const mockEvent = { clientX: 150, clientY: 250 };
260321
const trackFormatMark = { type: { name: 'trackFormat' }, attrs: { id: 'track-format-1' } };

‎packages/super-editor/src/components/context-menu/utils.js‎

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from '@extensions/track-changes/permission-helpers.js';
1010
import { isList } from '@core/commands/list-helpers';
1111
import { isCellSelection } from '@extensions/table/tableHelpers/isCellSelection.js';
12+
import { hasExpandedSelection } from '@utils/selectionUtils.js';
1213
import { selectedRect } from 'prosemirror-tables';
1314
/**
1415
* Get props by item id
@@ -85,9 +86,7 @@ export async function getEditorContext(editor, event) {
8586

8687
const state = editor.state;
8788
if (!state) return null;
88-
89-
const { from, to, empty } = state.selection;
90-
const selectedText = !empty ? state.doc.textBetween(from, to) : '';
89+
const { from } = state.selection;
9190

9291
let pos = null;
9392
let node = null;
@@ -106,6 +105,10 @@ export async function getEditorContext(editor, event) {
106105
node = state.doc.nodeAt(pos);
107106
}
108107

108+
const selection = getContextSelection({ editor, state, pos, event });
109+
const hasSelection = hasExpandedSelection(selection);
110+
const selectedText = hasSelection ? state.doc.textBetween(selection.from, selection.to) : '';
111+
109112
// Don't read clipboard proactively to avoid permission prompts
110113
// Clipboard will be read only when user actually clicks "Paste"
111114
const clipboardContent = {
@@ -166,10 +169,15 @@ export async function getEditorContext(editor, event) {
166169
const isTrackedChange =
167170
activeMarks.includes('trackInsert') || activeMarks.includes('trackDelete') || activeMarks.includes('trackFormat');
168171

169-
const trackedChanges =
170-
event && pos !== null
172+
// If there is an expanded selection and the right-click happened inside
173+
// that selection, use collectTrackedChanges for the full selection range
174+
const shouldUseSelectionTrackedChanges =
175+
event && pos !== null ? hasExpandedSelection(selection) && selectionContainsPos(selection, pos) : hasSelection;
176+
const trackedChanges = shouldUseSelectionTrackedChanges
177+
? collectTrackedChanges({ state, from: selection.from, to: selection.to })
178+
: event && pos !== null
171179
? collectTrackedChangesForContext({ state, pos, trackedChangeId })
172-
: collectTrackedChanges({ state, from, to });
180+
: collectTrackedChanges({ state, from: selection.from, to: selection.to });
173181

174182
const cursorCoords = pos !== null ? editor.coordsAtPos?.(pos) : null;
175183
const cursorPosition = cursorCoords
@@ -183,9 +191,9 @@ export async function getEditorContext(editor, event) {
183191

184192
return {
185193
selectedText,
186-
hasSelection: !empty,
187-
selectionStart: from,
188-
selectionEnd: to,
194+
hasSelection,
195+
selectionStart: selection.from,
196+
selectionEnd: selection.to,
189197
isInTable,
190198
isInList,
191199
isInSectionNode,
@@ -210,6 +218,29 @@ export async function getEditorContext(editor, event) {
210218
};
211219
}
212220

221+
function selectionContainsPos(selection, pos) {
222+
return hasExpandedSelection(selection) && Number.isFinite(pos) && pos >= selection.from && pos <= selection.to;
223+
}
224+
225+
function getContextSelection({ editor, state, pos, event }) {
226+
const currentSelection = state.selection;
227+
const preservedSelection = editor?.options?.preservedSelection ?? editor?.options?.lastSelection;
228+
229+
if (hasExpandedSelection(currentSelection)) {
230+
return currentSelection;
231+
}
232+
233+
if (!hasExpandedSelection(preservedSelection)) {
234+
return currentSelection;
235+
}
236+
237+
if (event) {
238+
return selectionContainsPos(preservedSelection, pos) ? preservedSelection : currentSelection;
239+
}
240+
241+
return preservedSelection;
242+
}
243+
213244
function computeCanUndo(editor, state) {
214245
if (typeof editor?.can === 'function') {
215246
try {

0 commit comments

Comments
 (0)