fix: Make the undo/redo API more ergonomic (#9573)

* fix: Make the undo/redo API more ergonomic

* fix: Update callers to use the new API

* chore: Fix lint
This commit is contained in:
Aaron Dodson
2026-04-21 11:36:48 -07:00
committed by GitHub
parent 03443975d3
commit f80253f5e9
8 changed files with 46 additions and 40 deletions
+2 -2
View File
@@ -47,7 +47,7 @@ export function registerUndo() {
return 'disabled';
},
callback(scope: Scope) {
scope.workspace!.undo(false);
scope.workspace!.undo();
},
scopeType: ContextMenuRegistry.ScopeType.WORKSPACE,
id: 'undoWorkspace',
@@ -71,7 +71,7 @@ export function registerRedo() {
return 'disabled';
},
callback(scope: Scope) {
scope.workspace!.undo(true);
scope.workspace!.redo();
},
scopeType: ContextMenuRegistry.ScopeType.WORKSPACE,
id: 'redoWorkspace',
+2 -2
View File
@@ -331,7 +331,7 @@ export function registerUndo() {
callback(workspace, e) {
// 'z' for undo 'Z' is for redo.
(workspace as WorkspaceSvg).hideChaff();
workspace.undo(false);
workspace.undo();
e.preventDefault();
return true;
},
@@ -367,7 +367,7 @@ export function registerRedo() {
callback(workspace, e) {
// 'z' for undo 'Z' is for redo.
(workspace as WorkspaceSvg).hideChaff();
workspace.undo(true);
workspace.redo();
e.preventDefault();
return true;
},
+8 -1
View File
@@ -728,7 +728,7 @@ export class Workspace {
*
* @param redo False if undo, true if redo.
*/
undo(redo: boolean) {
undo(redo = false) {
const inputStack = redo ? this.redoStack_ : this.undoStack_;
const outputStack = redo ? this.undoStack_ : this.redoStack_;
const inputEvent = inputStack.pop();
@@ -762,6 +762,13 @@ export class Workspace {
}
}
/**
* Redoes the previous action.
*/
redo() {
this.undo(true);
}
/** Clear the undo/redo stacks. */
clearUndo() {
this.undoStack_.length = 0;
@@ -88,7 +88,7 @@ suite('Comment Deserialization', function () {
this.block.checkAndDelete();
assert.equal(this.workspace.getAllBlocks().length, 0);
// Undo.
this.workspace.undo(false);
this.workspace.undo();
assert.equal(this.workspace.getAllBlocks().length, 1);
// Check comment.
assertComment(this.workspace, 'test text');
@@ -97,12 +97,12 @@ suite('Comment Deserialization', function () {
// Create block.
this.block = createBlock(this.workspace);
// Undo & undo.
this.workspace.undo(false);
this.workspace.undo(false);
this.workspace.undo();
this.workspace.undo();
assert.equal(this.workspace.getAllBlocks().length, 0);
// Redo & redo.
this.workspace.undo(true);
this.workspace.undo(true);
this.workspace.redo();
this.workspace.redo();
assert.equal(this.workspace.getAllBlocks().length, 1);
// Check comment.
assertComment(this.workspace, 'test text');
+6 -6
View File
@@ -174,7 +174,7 @@ suite('Comments', function () {
assert.isNotNull(block.getIcon(Blockly.icons.IconType.COMMENT));
assert.equal(block.getCommentText(), '');
this.workspace.undo(false);
this.workspace.undo();
assert.isUndefined(block.getIcon(Blockly.icons.IconType.COMMENT));
assert.isNull(block.getCommentText());
@@ -183,8 +183,8 @@ suite('Comments', function () {
test('Adding an empty comment can be redone', function () {
const block = this.workspace.newBlock('empty_block');
block.setCommentText('');
this.workspace.undo(false);
this.workspace.undo(true);
this.workspace.undo();
this.workspace.redo();
assert.isNotNull(block.getIcon(Blockly.icons.IconType.COMMENT));
assert.equal(block.getCommentText(), '');
@@ -196,7 +196,7 @@ suite('Comments', function () {
assert.isNotNull(block.getIcon(Blockly.icons.IconType.COMMENT));
assert.equal(block.getCommentText(), 'hey there');
this.workspace.undo(false);
this.workspace.undo();
assert.isUndefined(block.getIcon(Blockly.icons.IconType.COMMENT));
assert.isNull(block.getCommentText());
@@ -205,8 +205,8 @@ suite('Comments', function () {
test('Adding a non-empty comment can be redone', function () {
const block = this.workspace.newBlock('empty_block');
block.setCommentText('hey there');
this.workspace.undo(false);
this.workspace.undo(true);
this.workspace.undo();
this.workspace.redo();
assert.isNotNull(block.getIcon(Blockly.icons.IconType.COMMENT));
assert.equal(block.getCommentText(), 'hey there');
@@ -93,7 +93,7 @@ suite('Context Menu Items', function () {
test('Enabled when something to redo', function () {
// Create a new block, then undo it, which means there is something to redo.
this.workspace.newBlock('text');
this.workspace.undo(false);
this.workspace.undo();
const precondition = this.redoOption.preconditionFn(this.scope);
assert.equal(
precondition,
@@ -105,7 +105,7 @@ suite('Context Menu Items', function () {
test('Redoes adding new block', function () {
// Add a new block, then undo it, then redo it.
this.workspace.newBlock('text');
this.workspace.undo(false);
this.workspace.undo();
assert.equal(this.workspace.getTopBlocks(false).length, 0);
this.redoOption.callback(this.scope);
assert.equal(
@@ -333,7 +333,6 @@ suite('Keyboard Shortcut Items', function () {
test('Simple', function () {
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.calledOnce(this.undoSpy);
sinon.assert.calledWith(this.undoSpy, false);
sinon.assert.calledOnce(this.hideChaffSpy);
});
// Do not undo if a drag is in progress.
@@ -351,7 +350,7 @@ suite('Keyboard Shortcut Items', function () {
suite('Redo', function () {
setup(function () {
this.redoSpy = sinon.spy(this.workspace, 'undo');
this.redoSpy = sinon.spy(this.workspace, 'redo');
this.hideChaffSpy = sinon.spy(
Blockly.WorkspaceSvg.prototype,
'hideChaff',
@@ -361,11 +360,10 @@ suite('Keyboard Shortcut Items', function () {
Blockly.utils.KeyCodes.CTRL_CMD,
Blockly.utils.KeyCodes.SHIFT,
]);
// Undo.
// Redo.
test('Simple', function () {
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.calledOnce(this.redoSpy);
sinon.assert.calledWith(this.redoSpy, true);
sinon.assert.calledOnce(this.hideChaffSpy);
});
// Do not redo if a drag is in progress.
@@ -1287,7 +1287,7 @@ export function testAWorkspace() {
assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
assert.isNull(this.variableMap.getVariableById('id2'));
this.workspace.undo(true);
this.workspace.redo();
// Expect that variable 'id2' is recreated
assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
@@ -1295,9 +1295,10 @@ export function testAWorkspace() {
this.workspace.undo();
this.workspace.undo();
assert.isNull(this.variableMap.getVariableById('id1'));
assert.isNull(this.variableMap.getVariableById('id2'));
this.workspace.undo(true);
this.workspace.redo();
// Expect that variable 'id1' is recreated
assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
@@ -1365,7 +1366,7 @@ export function testAWorkspace() {
assert.isNull(this.variableMap.getVariableById('id1'));
assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
// Expect that both variables are deleted
assert.isNull(this.variableMap.getVariableById('id1'));
@@ -1378,7 +1379,7 @@ export function testAWorkspace() {
assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
// Expect that variable 'id2' is recreated
assert.isNull(this.variableMap.getVariableById('id1'));
@@ -1402,7 +1403,7 @@ export function testAWorkspace() {
assert.isNull(this.variableMap.getVariableById('id1'));
assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
// Expect that both variables are deleted
assert.equal(this.workspace.getTopBlocks(false).length, 0);
@@ -1418,7 +1419,7 @@ export function testAWorkspace() {
assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
// Expect that variable 'id2' is recreated
assertBlockVarModelName(this.workspace, 0, 'name2');
@@ -1441,7 +1442,7 @@ export function testAWorkspace() {
this.clock.runAll();
assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'name2', 'type1', 'id1');
});
@@ -1457,7 +1458,7 @@ export function testAWorkspace() {
assertBlockVarModelName(this.workspace, 0, 'name1');
assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertBlockVarModelName(this.workspace, 0, 'name2');
assertVariableValues(this.variableMap, 'name2', 'type1', 'id1');
@@ -1472,7 +1473,7 @@ export function testAWorkspace() {
this.clock.runAll();
assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'Name1', 'type1', 'id1');
});
@@ -1488,7 +1489,7 @@ export function testAWorkspace() {
assertBlockVarModelName(this.workspace, 0, 'name1');
assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertBlockVarModelName(this.workspace, 0, 'Name1');
assertVariableValues(this.variableMap, 'Name1', 'type1', 'id1');
@@ -1506,7 +1507,7 @@ export function testAWorkspace() {
assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
assertVariableValues(this.variableMap, 'name2', 'type1', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'name2', 'type1', 'id2');
assert.isNull(this.variableMap.getVariableById('id1'));
@@ -1530,7 +1531,7 @@ export function testAWorkspace() {
assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
assertVariableValues(this.variableMap, 'name2', 'type1', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'name2', 'type1', 'id2');
assert.isNull(this.variableMap.getVariableById('id1'));
@@ -1547,7 +1548,7 @@ export function testAWorkspace() {
assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
assertVariableValues(this.variableMap, 'name2', 'type1', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'Name2', 'type1', 'id2');
assert.isNull(this.variableMap.getVariable('name1'));
@@ -1567,7 +1568,7 @@ export function testAWorkspace() {
assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
assertVariableValues(this.variableMap, 'name2', 'type1', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'Name2', 'type1', 'id2');
assert.isNull(this.variableMap.getVariableById('id1'));
@@ -1586,7 +1587,7 @@ export function testAWorkspace() {
assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
assertVariableValues(this.variableMap, 'name2', 'type2', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'name2', 'type1', 'id1');
assertVariableValues(this.variableMap, 'name2', 'type2', 'id2');
@@ -1606,7 +1607,7 @@ export function testAWorkspace() {
assertBlockVarModelName(this.workspace, 0, 'name1');
assertBlockVarModelName(this.workspace, 1, 'name2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'name2', 'type1', 'id1');
assertVariableValues(this.variableMap, 'name2', 'type2', 'id2');
@@ -1625,7 +1626,7 @@ export function testAWorkspace() {
assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'Name2', 'type1', 'id1');
assertVariableValues(this.variableMap, 'name2', 'type2', 'id2');
@@ -1645,7 +1646,7 @@ export function testAWorkspace() {
assertBlockVarModelName(this.workspace, 0, 'name1');
assertBlockVarModelName(this.workspace, 1, 'name2');
this.workspace.undo(true);
this.workspace.redo();
this.clock.runAll();
assertVariableValues(this.variableMap, 'Name2', 'type1', 'id1');
assertVariableValues(this.variableMap, 'name2', 'type2', 'id2');