Skip to content

Commit 31cb404

Browse files
Desktop: Fixes #13745: Prevent cut events from being merged with other actions in the undo history (#13791)
1 parent dba3a3f commit 31cb404

File tree

7 files changed

+136
-31
lines changed

7 files changed

+136
-31
lines changed

.eslintignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,7 @@ packages/app-desktop/integration-tests/util/evaluateWithRetry.js
563563
packages/app-desktop/integration-tests/util/extendedExpect.js
564564
packages/app-desktop/integration-tests/util/getImageSourceSize.js
565565
packages/app-desktop/integration-tests/util/getMainWindow.js
566+
packages/app-desktop/integration-tests/util/mockClipboard.js
566567
packages/app-desktop/integration-tests/util/retryOnFailure.js
567568
packages/app-desktop/integration-tests/util/setDarkMode.js
568569
packages/app-desktop/integration-tests/util/setFilePickerResponse.js
@@ -1007,6 +1008,7 @@ packages/editor/CodeMirror/CodeMirrorControl.js
10071008
packages/editor/CodeMirror/configFromSettings.js
10081009
packages/editor/CodeMirror/createEditor.test.js
10091010
packages/editor/CodeMirror/createEditor.js
1011+
packages/editor/CodeMirror/editorCommands/cutOrCopyText.js
10101012
packages/editor/CodeMirror/editorCommands/duplicateLine.test.js
10111013
packages/editor/CodeMirror/editorCommands/duplicateLine.js
10121014
packages/editor/CodeMirror/editorCommands/editorCommands.js

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ packages/app-desktop/integration-tests/util/evaluateWithRetry.js
535535
packages/app-desktop/integration-tests/util/extendedExpect.js
536536
packages/app-desktop/integration-tests/util/getImageSourceSize.js
537537
packages/app-desktop/integration-tests/util/getMainWindow.js
538+
packages/app-desktop/integration-tests/util/mockClipboard.js
538539
packages/app-desktop/integration-tests/util/retryOnFailure.js
539540
packages/app-desktop/integration-tests/util/setDarkMode.js
540541
packages/app-desktop/integration-tests/util/setFilePickerResponse.js
@@ -979,6 +980,7 @@ packages/editor/CodeMirror/CodeMirrorControl.js
979980
packages/editor/CodeMirror/configFromSettings.js
980981
packages/editor/CodeMirror/createEditor.test.js
981982
packages/editor/CodeMirror/createEditor.js
983+
packages/editor/CodeMirror/editorCommands/cutOrCopyText.js
982984
packages/editor/CodeMirror/editorCommands/duplicateLine.test.js
983985
packages/editor/CodeMirror/editorCommands/duplicateLine.js
984986
packages/editor/CodeMirror/editorCommands/editorCommands.js

packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.tsx

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { _ } from '@joplin/lib/locale';
1313
import bridge from '../../../../../services/bridge';
1414
import shim from '@joplin/lib/shim';
1515
import { MarkupToHtml } from '@joplin/renderer';
16-
const { clipboard } = require('electron');
16+
import { clipboard } from 'electron';
1717
import { reg } from '@joplin/lib/registry';
1818
import ErrorBoundary from '../../../../ErrorBoundary';
1919
import { EditorKeymap, EditorLanguageType, EditorSettings, SearchState, UserEventSource } from '@joplin/editor/types';
@@ -94,41 +94,13 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
9494

9595
const editorCutText = useCallback(() => {
9696
if (editorRef.current) {
97-
const selections = editorRef.current.getSelections();
98-
if (selections.length > 0 && selections[0]) {
99-
clipboard.writeText(selections[0]);
100-
// Easy way to wipe out just the first selection
101-
selections[0] = '';
102-
editorRef.current.replaceSelections(selections);
103-
} else {
104-
const cursor = editorRef.current.getCursor();
105-
const line = editorRef.current.getLine(cursor.line);
106-
clipboard.writeText(`${line}\n`);
107-
const startLine = editorRef.current.getCursor('head');
108-
startLine.ch = 0;
109-
const endLine = {
110-
line: startLine.line + 1,
111-
ch: 0,
112-
};
113-
editorRef.current.replaceRange('', startLine, endLine);
114-
}
97+
editorRef.current.cutText(text => clipboard.writeText(text));
11598
}
11699
}, []);
117100

118101
const editorCopyText = useCallback(() => {
119102
if (editorRef.current) {
120-
const selections = editorRef.current.getSelections();
121-
122-
// Handle the case when there is a selection - copy the selection to the clipboard
123-
// When there is no selection, the selection array contains an empty string.
124-
if (selections.length > 0 && selections[0]) {
125-
clipboard.writeText(selections[0]);
126-
} else {
127-
// This is the case when there is no selection - copy the current line to the clipboard
128-
const cursor = editorRef.current.getCursor();
129-
const line = editorRef.current.getLine(cursor.line);
130-
clipboard.writeText(line);
131-
}
103+
editorRef.current.copyText(text => clipboard.writeText(text));
132104
}
133105
}, []);
134106

packages/app-desktop/integration-tests/markdownEditor.spec.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import setFilePickerResponse from './util/setFilePickerResponse';
66
import activateMainMenuItem from './util/activateMainMenuItem';
77
import setSettingValue from './util/setSettingValue';
88
import { toForwardSlashes } from '@joplin/utils/path';
9+
import mockClipboard from './util/mockClipboard';
910

1011

1112
test.describe('markdownEditor', () => {
@@ -337,5 +338,48 @@ test.describe('markdownEditor', () => {
337338
// Should show the legacy editor
338339
await expect(mainWindow.locator('.rli-editor .CodeMirror5')).toBeVisible();
339340
});
341+
342+
test('should support the textCopy command', async ({ electronApp, mainWindow }) => {
343+
const mainScreen = await new MainScreen(mainWindow).setup();
344+
await mainScreen.waitFor();
345+
346+
await mainScreen.createNewNote('Test copy');
347+
const noteEditor = mainScreen.noteEditor;
348+
await noteEditor.focusCodeMirrorEditor();
349+
await mainWindow.keyboard.type('Test content.');
350+
351+
const { expectClipboardToMatch } = await mockClipboard(electronApp, 'original');
352+
353+
await mainScreen.goToAnything.runCommand(electronApp, 'textCopy');
354+
await expectClipboardToMatch('Test content.\n');
355+
});
356+
357+
test('should support the textCut and textPaste commands', async ({ electronApp, mainWindow }) => {
358+
const mainScreen = await new MainScreen(mainWindow).setup();
359+
await mainScreen.waitFor();
360+
361+
await mainScreen.createNewNote('Test paste');
362+
const { expectClipboardToMatch } = await mockClipboard(electronApp, 'test!');
363+
await expectClipboardToMatch('test!');
364+
365+
// Should paste text using the textPaste command
366+
const goToAnything = mainScreen.goToAnything;
367+
await goToAnything.runCommand(electronApp, 'textPaste');
368+
const noteEditor = mainScreen.noteEditor;
369+
await noteEditor.expectToHaveText('test!');
370+
371+
// Should cut text using the textCut command
372+
await mainScreen.createNewNote('Test cut');
373+
await noteEditor.focusCodeMirrorEditor();
374+
await mainWindow.keyboard.type('Test (new content!)');
375+
376+
await goToAnything.runCommand(electronApp, 'textCut');
377+
await noteEditor.expectToHaveText('\n');
378+
await expectClipboardToMatch('Test (new content!)\n');
379+
380+
// Should paste the content again with textPaste
381+
await goToAnything.runCommand(electronApp, 'textPaste');
382+
await noteEditor.expectToHaveText(/^Test \(new content!\)[\n]+/);
383+
});
340384
});
341385

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { ElectronApplication } from '@playwright/test';
2+
import { expect } from './test';
3+
import getMainWindow from './getMainWindow';
4+
5+
// Currently only supports mocking reading/writing text
6+
const mockClipboard = async (electronApp: ElectronApplication, clipboardText: string) => {
7+
const mainWindow = await getMainWindow(electronApp);
8+
await mainWindow.evaluate(async (clipboardText) => {
9+
const { clipboard } = require('electron');
10+
clipboard.writeText = (text: string) => {
11+
clipboardText = text;
12+
};
13+
clipboard.readText = () => {
14+
return clipboardText;
15+
};
16+
}, clipboardText);
17+
18+
return {
19+
expectClipboardToMatch: async (text: string) => {
20+
await expect.poll(async () => {
21+
return await mainWindow.evaluate(() => {
22+
return require('electron').clipboard.readText();
23+
});
24+
}).toBe(text);
25+
},
26+
};
27+
};
28+
29+
export default mockClipboard;

packages/editor/CodeMirror/CodeMirrorControl.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import jumpToHash from './editorCommands/jumpToHash';
1616
import { resetImageResourceEffect } from './extensions/rendering/renderBlockImages';
1717
import Logger from '@joplin/utils/Logger';
1818
import { searchChangeSourceEffect } from './extensions/searchExtension';
19+
import cutOrCopyText, { ClipboardAction } from './editorCommands/cutOrCopyText';
1920

2021
const logger = Logger.create('CodeMirrorControl');
2122

@@ -260,6 +261,14 @@ export default class CodeMirrorControl extends CodeMirror5Emulation implements E
260261
this._callbacks.onRemove();
261262
}
262263

264+
public cutText(writeClipboard: (text: string)=> void) {
265+
return cutOrCopyText(writeClipboard, ClipboardAction.Cut)(this.editor);
266+
}
267+
268+
public copyText(writeClipboard: (text: string)=> void) {
269+
return cutOrCopyText(writeClipboard, ClipboardAction.Copy)(this.editor);
270+
}
271+
263272
//
264273
// CodeMirror-specific methods
265274
//
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { isolateHistory } from '@codemirror/commands';
2+
import { Command } from '@codemirror/view';
3+
4+
export enum ClipboardAction {
5+
Cut = 'cut',
6+
Copy = 'copy',
7+
}
8+
9+
type OnWriteClipboard = (text: string)=> void;
10+
const cutOrCopyText = (onWriteClipboard: OnWriteClipboard, action: ClipboardAction): Command => (view) => {
11+
const state = view.state;
12+
const selections = state.selection.ranges.map(range => (
13+
state.sliceDoc(range.from, range.to)
14+
));
15+
const nonEmptySelections = selections.filter(s => !!s);
16+
17+
const cutTransactions = [];
18+
if (nonEmptySelections.length > 0) {
19+
onWriteClipboard(nonEmptySelections.join('\n'));
20+
21+
cutTransactions.push(state.replaceSelection(''));
22+
} else {
23+
const selectedLine = state.doc.lineAt(state.selection.main.anchor);
24+
onWriteClipboard(`${selectedLine.text}\n`);
25+
26+
cutTransactions.push({
27+
changes: [{
28+
from: selectedLine.from,
29+
to: Math.min(selectedLine.to + 1, state.doc.length),
30+
insert: '',
31+
}],
32+
});
33+
}
34+
35+
cutTransactions.push({
36+
annotations: [isolateHistory.of('full')],
37+
userEvent: 'delete.cut',
38+
});
39+
40+
if (action === ClipboardAction.Cut) {
41+
view.dispatch(...cutTransactions);
42+
}
43+
44+
return true;
45+
};
46+
47+
export default cutOrCopyText;

0 commit comments

Comments
 (0)