Skip to content

Commit 5fbe29e

Browse files
rchiodorchiodo
committed
Fix for 7999 - CTRL+Z should not undo at the top level (#8571)
1 parent 24b60ec commit 5fbe29e

File tree

6 files changed

+91
-28
lines changed

6 files changed

+91
-28
lines changed

‎news/2 Fixes/7999.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
CTRL+Z is deleting cells. It should only undo changes inside of the code for a cell. 'Z' and 'SHIFT+Z' are for undoing/redoing cell adds/moves.

‎src/client/datascience/interactive-common/interactiveWindowTypes.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ export enum NativeCommandType {
9797
InsertBelow,
9898
MoveCellDown,
9999
MoveCellUp,
100+
Redo,
100101
Run,
101102
RunAbove,
102103
RunAll,

‎src/datascience-ui/interactive-common/mainStateController.ts‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -267,31 +267,35 @@ export class MainStateController implements IMessageHandler {
267267
}
268268

269269
public redo = () => {
270-
// Pop one off of our redo stack and update our undo
271-
const cells = this.pendingState.redoStack[this.pendingState.redoStack.length - 1];
272-
const redoStack = this.pendingState.redoStack.slice(0, this.pendingState.redoStack.length - 1);
273-
const undoStack = this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs);
274-
this.sendMessage(InteractiveWindowMessages.Redo);
275-
this.setState({
276-
cellVMs: cells,
277-
undoStack: undoStack,
278-
redoStack: redoStack,
279-
skipNextScroll: true
280-
});
270+
if (this.pendingState.redoStack.length > 0) {
271+
// Pop one off of our redo stack and update our undo
272+
const cells = this.pendingState.redoStack[this.pendingState.redoStack.length - 1];
273+
const redoStack = this.pendingState.redoStack.slice(0, this.pendingState.redoStack.length - 1);
274+
const undoStack = this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs);
275+
this.sendMessage(InteractiveWindowMessages.Redo);
276+
this.setState({
277+
cellVMs: cells,
278+
undoStack: undoStack,
279+
redoStack: redoStack,
280+
skipNextScroll: true
281+
});
282+
}
281283
}
282284

283285
public undo = () => {
284-
// Pop one off of our undo stack and update our redo
285-
const cells = this.pendingState.undoStack[this.pendingState.undoStack.length - 1];
286-
const undoStack = this.pendingState.undoStack.slice(0, this.pendingState.undoStack.length - 1);
287-
const redoStack = this.pushStack(this.pendingState.redoStack, this.pendingState.cellVMs);
288-
this.sendMessage(InteractiveWindowMessages.Undo);
289-
this.setState({
290-
cellVMs: cells,
291-
undoStack: undoStack,
292-
redoStack: redoStack,
293-
skipNextScroll: true
294-
});
286+
if (this.pendingState.undoStack.length > 0) {
287+
// Pop one off of our undo stack and update our redo
288+
const cells = this.pendingState.undoStack[this.pendingState.undoStack.length - 1];
289+
const undoStack = this.pendingState.undoStack.slice(0, this.pendingState.undoStack.length - 1);
290+
const redoStack = this.pushStack(this.pendingState.redoStack, this.pendingState.cellVMs);
291+
this.sendMessage(InteractiveWindowMessages.Undo);
292+
this.setState({
293+
cellVMs: cells,
294+
undoStack: undoStack,
295+
redoStack: redoStack,
296+
skipNextScroll: true
297+
});
298+
}
295299
}
296300

297301
public deleteCell = (cellId: string) => {

‎src/datascience-ui/native-editor/nativeCell.tsx‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -325,10 +325,17 @@ export class NativeCell extends React.Component<INativeCellProps> {
325325
}
326326
break;
327327
case 'z':
328-
if (!this.isFocused() && this.props.stateController.canUndo()) {
329-
e.stopPropagation();
330-
this.props.stateController.undo();
331-
this.props.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
328+
case 'Z':
329+
if (!this.isFocused()) {
330+
if (e.shiftKey && !e.ctrlKey && !e.altKey && this.props.stateController.canRedo()) {
331+
e.stopPropagation();
332+
this.props.stateController.redo();
333+
this.props.stateController.sendCommand(NativeCommandType.Redo, 'keyboard');
334+
} else if (!e.shiftKey && !e.altKey && !e.ctrlKey && this.props.stateController.canUndo()) {
335+
e.stopPropagation();
336+
this.props.stateController.undo();
337+
this.props.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
338+
}
332339
}
333340
break;
334341

‎src/datascience-ui/native-editor/nativeEditor.tsx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,18 @@ export class NativeEditor extends React.Component<INativeEditorProps, IMainState
365365
}
366366
break;
367367
}
368+
case 'z':
369+
case 'Z':
370+
if (event.shiftKey && !event.ctrlKey && !event.altKey && this.stateController.canRedo()) {
371+
event.stopPropagation();
372+
this.stateController.redo();
373+
this.stateController.sendCommand(NativeCommandType.Redo, 'keyboard');
374+
} else if (!event.shiftKey && !event.altKey && !event.ctrlKey && this.stateController.canUndo()) {
375+
event.stopPropagation();
376+
this.stateController.undo();
377+
this.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
378+
}
379+
break;
368380
default:
369381
break;
370382
}

‎src/test/datascience/nativeEditor.functional.test.tsx‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { Disposable, TextDocument, TextEditor, Uri, WindowState } from 'vscode';
1414

1515
import { IApplicationShell, IDocumentManager } from '../../client/common/application/types';
1616
import { IFileSystem } from '../../client/common/platform/types';
17-
import { createDeferred } from '../../client/common/utils/async';
17+
import { createDeferred, sleep, waitForPromise } from '../../client/common/utils/async';
1818
import { createTemporaryFile } from '../../client/common/utils/fs';
1919
import { noop } from '../../client/common/utils/misc';
2020
import { Identifiers } from '../../client/datascience/constants';
@@ -940,10 +940,14 @@ for _ in range(50):
940940
// Add, then undo, keep doing at least 3 times and confirm it works as expected.
941941
for (let i = 0; i < 3; i += 1) {
942942
// Add a new cell
943-
let update = waitForUpdate(wrapper, NativeEditor, 1);
943+
let update = waitForMessage(ioc, InteractiveWindowMessages.FocusedCellEditor);
944944
simulateKeyPressOnCell(0, { code: 'a' });
945945
await update;
946946

947+
// Wait a bit for the time out to try and set focus a second time (this will be
948+
// fixed when we switch to redux)
949+
await sleep(100);
950+
947951
// There should be 4 cells and first cell is focused.
948952
assert.equal(isCellSelected(wrapper, 'NativeCell', 0), false);
949953
assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false);
@@ -957,6 +961,40 @@ for _ in range(50):
957961
await update;
958962
assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true);
959963

964+
// Press 'ctrl+z'. This should do nothing
965+
update = waitForUpdate(wrapper, NativeEditor, 1);
966+
simulateKeyPressOnCell(0, { code: 'z', ctrlKey: true });
967+
await waitForPromise(update, 100);
968+
969+
// There should be 4 cells and first cell is selected.
970+
assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true);
971+
assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false);
972+
assert.equal(isCellFocused(wrapper, 'NativeCell', 0), false);
973+
assert.equal(isCellFocused(wrapper, 'NativeCell', 1), false);
974+
assert.equal(wrapper.find('NativeCell').length, 4);
975+
976+
// Press 'z' to undo.
977+
update = waitForUpdate(wrapper, NativeEditor, 1);
978+
simulateKeyPressOnCell(0, { code: 'z' });
979+
await update;
980+
981+
// There should be 3 cells and first cell is selected & nothing focused.
982+
assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true);
983+
assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false);
984+
assert.equal(wrapper.find('NativeCell').length, 3);
985+
986+
// Press 'shift+z' to redo
987+
update = waitForUpdate(wrapper, NativeEditor, 1);
988+
simulateKeyPressOnCell(0, { code: 'z', shiftKey: true });
989+
await update;
990+
991+
// There should be 4 cells and first cell is selected.
992+
assert.equal(isCellSelected(wrapper, 'NativeCell', 0), true);
993+
assert.equal(isCellSelected(wrapper, 'NativeCell', 1), false);
994+
assert.equal(isCellFocused(wrapper, 'NativeCell', 0), false);
995+
assert.equal(isCellFocused(wrapper, 'NativeCell', 1), false);
996+
assert.equal(wrapper.find('NativeCell').length, 4);
997+
960998
// Press 'z' to undo.
961999
update = waitForUpdate(wrapper, NativeEditor, 1);
9621000
simulateKeyPressOnCell(0, { code: 'z' });

0 commit comments

Comments
 (0)