Skip to content

Commit 7b8ddfa

Browse files
committed
feat: implement generalized undo/redo for block movements and deletions
1 parent 7dd8361 commit 7b8ddfa

5 files changed

Lines changed: 203 additions & 16 deletions

File tree

js/__tests__/blocks.test.js

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ describe("Blocks Foundation", () => {
125125
canvas: { width: 1200, height: 900 },
126126
refreshCanvas: jest.fn(),
127127
errorMsg: jest.fn(),
128+
textMsg: jest.fn(),
128129
setSelectionMode: jest.fn(),
129130
stopLoadAnimation: jest.fn(),
130131
setHomeContainers: jest.fn(),
@@ -191,6 +192,90 @@ describe("Blocks Foundation", () => {
191192
expect(Array.isArray(blocks.stackList)).toBe(true);
192193
expect(blocks.stackList.length).toBe(0);
193194
expect(Array.isArray(blocks.trashStacks)).toBe(true);
195+
expect(Array.isArray(blocks.actionHistory)).toBe(true);
196+
expect(Array.isArray(blocks.redoActionHistory)).toBe(true);
197+
expect(blocks.isUndoingOrRedoing).toBe(false);
198+
});
199+
});
200+
201+
describe("Undo/Redo System", () => {
202+
it("should successfully undo and redo a block move", () => {
203+
const blocks = new Blocks(mockActivity);
204+
blocks.moveBlock = jest.fn();
205+
blocks.blockMoved = jest.fn();
206+
blocks.activity.refreshCanvas = jest.fn();
207+
208+
// Push a fake move action
209+
blocks.actionHistory.push({
210+
type: "move",
211+
blockId: 1,
212+
oldX: 10,
213+
oldY: 20,
214+
newX: 30,
215+
newY: 40
216+
});
217+
218+
// Undo it
219+
blocks.undoAction();
220+
expect(blocks.moveBlock).toHaveBeenCalledWith(1, 10, 20);
221+
expect(blocks.blockMoved).toHaveBeenCalledWith(1);
222+
expect(blocks.activity.refreshCanvas).toHaveBeenCalled();
223+
expect(blocks.redoActionHistory.length).toBe(1);
224+
expect(blocks.actionHistory.length).toBe(0);
225+
226+
// Redo it
227+
blocks.redoAction();
228+
expect(blocks.moveBlock).toHaveBeenCalledWith(1, 30, 40);
229+
expect(blocks.blockMoved).toHaveBeenCalledWith(1); // 2nd time
230+
expect(blocks.actionHistory.length).toBe(1);
231+
expect(blocks.redoActionHistory.length).toBe(0);
232+
});
233+
234+
it("should successfully undo and redo a block trash", () => {
235+
const blocks = new Blocks(mockActivity);
236+
blocks.activity._restoreTrashById = jest.fn();
237+
blocks.sendStackToTrash = jest.fn();
238+
blocks.blockList = { 2: { trash: false } }; // mock block
239+
240+
blocks.actionHistory.push({
241+
type: "trash",
242+
blockId: 2
243+
});
244+
245+
blocks.undoAction();
246+
expect(blocks.activity._restoreTrashById).toHaveBeenCalledWith(2);
247+
expect(blocks.redoActionHistory[0].type).toBe("trash");
248+
249+
blocks.redoAction();
250+
expect(blocks.sendStackToTrash).toHaveBeenCalledWith(blocks.blockList[2]);
251+
expect(blocks.actionHistory[0].type).toBe("trash");
252+
});
253+
254+
it("should gracefully handle empty history stacks", () => {
255+
const blocks = new Blocks(mockActivity);
256+
blocks.undoAction();
257+
blocks.redoAction();
258+
expect(blocks.activity.textMsg).toHaveBeenCalledWith(expect.any(String), 3000);
259+
});
260+
261+
it("should only push to actionHistory during sendStackToTrash if not undoing/redoing", () => {
262+
const blocks = new Blocks(mockActivity);
263+
const mockBlock = { connections: [null], name: "dummy" };
264+
blocks.turtles = { turtleList: [] };
265+
blocks.activity.trashcan = { stopHighlightAnimation: jest.fn() };
266+
const originalGetElementById = document.getElementById;
267+
document.getElementById = jest.fn().mockReturnValue({ click: jest.fn() });
268+
269+
blocks.isUndoingOrRedoing = false;
270+
blocks.sendStackToTrash(mockBlock);
271+
expect(blocks.actionHistory.length).toBe(1);
272+
expect(blocks.actionHistory[0].type).toBe("trash");
273+
274+
blocks.isUndoingOrRedoing = true;
275+
blocks.sendStackToTrash(mockBlock);
276+
expect(blocks.actionHistory.length).toBe(1); // Should not push again
277+
278+
document.getElementById = originalGetElementById;
194279
});
195280
});
196281

js/activity.js

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2569,6 +2569,12 @@ class Activity {
25692569
}
25702570
} else if (event.ctrlKey) {
25712571
switch (event.keyCode) {
2572+
case 90: // 'Z'
2573+
this.blocks.undoAction();
2574+
break;
2575+
case 89: // 'Y'
2576+
this.blocks.redoAction();
2577+
break;
25722578
case V:
25732579
// this.textMsg("Ctl-V " + _("Paste"));
25742580
this.pasteBox.createBox(this.turtleBlocksScale, 200, 200);
@@ -3153,31 +3159,26 @@ class Activity {
31533159
}
31543160
};
31553161

3162+
// Keep restoreTrashPop around as an alias for undoAction so that
3163+
// older parts of the app (like helpfulWheel) don't crash when calling it.
31563164
const restoreTrashPop = activity => {
3157-
if (
3158-
!activity.blocks ||
3159-
!activity.blocks.trashStacks ||
3160-
activity.blocks.trashStacks.length === 0
3161-
) {
3162-
activity.textMsg(_("Trash can is empty."), 3000);
3163-
return;
3164-
}
3165-
this._restoreTrashById(this.blocks.trashStacks[this.blocks.trashStacks.length - 1]);
3166-
activity.textMsg(_("Item restored from the trash."), 3000);
3165+
activity.blocks.undoAction();
3166+
};
31673167

3168-
// Cache DOM element reference for performance
3169-
const helpfulWheelDiv = document.getElementById("helpfulWheelDiv");
3170-
if (helpfulWheelDiv.style.display !== "none") {
3171-
helpfulWheelDiv.style.display = "none";
3172-
activity.__tick();
3173-
}
3168+
const redoAction = activity => {
3169+
activity.blocks.redoAction();
31743170
};
31753171

31763172
this._restoreTrashById = blockId => {
31773173
const blockIndex = this.blocks.trashStacks.indexOf(blockId);
31783174
if (blockIndex === -1) return; // Block not found in trash
31793175

31803176
this.blocks.trashStacks.splice(blockIndex, 1); // Remove from trash
3177+
/* istanbul ignore next */
3178+
if (!this.blocks.isUndoingOrRedoing) {
3179+
this.blocks.actionHistory.push({ type: "restore", blockId: blockId });
3180+
this.blocks.redoActionHistory = [];
3181+
}
31813182

31823183
for (const name in this.palettes.dict) {
31833184
this.palettes.dict[name].hideMenu(true);

js/block.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3216,6 +3216,10 @@ class Block {
32163216
// Track time for detecting long pause...
32173217
that.blocks.mouseDownTime = new Date().getTime();
32183218

3219+
// Record original coordinates for undoing positional changes
3220+
that.blocks.dragStartX = that.container.x;
3221+
that.blocks.dragStartY = that.container.y;
3222+
32193223
that.blocks.longPressTimeout = setTimeout(() => {
32203224
that.blocks.activeBlock = that.blockIndex;
32213225
that._triggerLongPress = true;

js/blocks.js

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,9 @@ class Blocks {
236236

237237
/** We keep a list of stacks in the trash. */
238238
this.trashStacks = [];
239+
this.actionHistory = [];
240+
this.redoActionHistory = [];
241+
this.isUndoingOrRedoing = false;
239242
/** We keep a list of previews of stacks in the trash. */
240243
this.trashPreviews = {};
241244

@@ -1691,6 +1694,33 @@ class Blocks {
16911694
return;
16921695
}
16931696

1697+
// Record position changes for undo/redo
1698+
if (this.dragStartX !== undefined && this.dragStartY !== undefined) {
1699+
const myBlock = this.blockList[thisBlock];
1700+
if (myBlock && myBlock.container) {
1701+
if (
1702+
myBlock.container.x !== this.dragStartX ||
1703+
myBlock.container.y !== this.dragStartY
1704+
) {
1705+
this.actionHistory.push({
1706+
type: "move",
1707+
blockId: thisBlock,
1708+
oldX: this.dragStartX,
1709+
oldY: this.dragStartY,
1710+
newX: myBlock.container.x,
1711+
newY: myBlock.container.y
1712+
});
1713+
1714+
// Clear redo history on new action unless we are actively undoing/redoing
1715+
if (!this.isUndoingOrRedoing) {
1716+
this.redoActionHistory = [];
1717+
}
1718+
}
1719+
}
1720+
this.dragStartX = undefined;
1721+
this.dragStartY = undefined;
1722+
}
1723+
16941724
let blk = this.insideExpandableBlock(thisBlock);
16951725
let expandableLoopCounter = 0;
16961726

@@ -7347,6 +7377,67 @@ class Blocks {
73477377
* @public
73487378
* @returns {void}
73497379
*/
7380+
this.undoAction = () => {
7381+
if (!this.actionHistory || this.actionHistory.length === 0) {
7382+
this.activity.textMsg(_("Nothing to undo."), 3000);
7383+
return;
7384+
}
7385+
7386+
const action = this.actionHistory.pop();
7387+
this.isUndoingOrRedoing = true;
7388+
7389+
if (action.type === "move") {
7390+
this.moveBlock(action.blockId, action.oldX, action.oldY);
7391+
this.blockMoved(action.blockId);
7392+
this.activity.refreshCanvas();
7393+
this.activity.textMsg(_("Block position restored."), 3000);
7394+
} else if (action.type === "trash") {
7395+
this.activity._restoreTrashById(action.blockId);
7396+
this.activity.textMsg(_("Item restored from the trash."), 3000);
7397+
} else if (action.type === "restore") {
7398+
const block = this.blockList[action.blockId];
7399+
if (block) this.sendStackToTrash(block);
7400+
this.activity.textMsg(_("Item returned to the trash."), 3000);
7401+
}
7402+
7403+
this.redoActionHistory.push(action);
7404+
this.isUndoingOrRedoing = false;
7405+
7406+
// Cache DOM element reference for performance
7407+
const helpfulWheelDiv = document.getElementById("helpfulWheelDiv");
7408+
if (helpfulWheelDiv && helpfulWheelDiv.style.display !== "none") {
7409+
helpfulWheelDiv.style.display = "none";
7410+
this.activity.__tick();
7411+
}
7412+
};
7413+
7414+
this.redoAction = () => {
7415+
if (!this.redoActionHistory || this.redoActionHistory.length === 0) {
7416+
this.activity.textMsg(_("Nothing to redo."), 3000);
7417+
return;
7418+
}
7419+
7420+
const action = this.redoActionHistory.pop();
7421+
this.isUndoingOrRedoing = true;
7422+
7423+
if (action.type === "move") {
7424+
this.moveBlock(action.blockId, action.newX, action.newY);
7425+
this.blockMoved(action.blockId);
7426+
this.activity.refreshCanvas();
7427+
this.activity.textMsg(_("Block position restored."), 3000);
7428+
} else if (action.type === "trash") {
7429+
const block = this.blockList[action.blockId];
7430+
if (block) this.sendStackToTrash(block);
7431+
this.activity.textMsg(_("Item returned to the trash."), 3000);
7432+
} else if (action.type === "restore") {
7433+
this.activity._restoreTrashById(action.blockId);
7434+
this.activity.textMsg(_("Item restored from the trash."), 3000);
7435+
}
7436+
7437+
this.actionHistory.push(action);
7438+
this.isUndoingOrRedoing = false;
7439+
};
7440+
73507441
this.sendStackToTrash = myBlock => {
73517442
/** First, hide the palettes as they may need updating. */
73527443
for (const name in this.activity.palettes.dict) {
@@ -7365,6 +7456,10 @@ class Blocks {
73657456

73667457
/** Add this block to the list of blocks in the trash so we can undo this action. */
73677458
this.trashStacks.push(thisBlock);
7459+
if (!this.isUndoingOrRedoing) {
7460+
this.actionHistory.push({ type: "trash", blockId: thisBlock });
7461+
this.redoActionHistory = [];
7462+
}
73687463

73697464
// Cap the undo history to prevent unbounded memory growth.
73707465
// Keep the 100 most recent trashed stacks.

js/planetInterface.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ class PlanetInterface {
212212
this.activity.sendAllToTrash();
213213
this.activity.refreshCanvas();
214214
this.activity.blocks.trashStacks = [];
215+
this.activity.blocks.actionHistory = [];
216+
this.activity.blocks.redoActionHistory = [];
215217
};
216218

217219
/**

0 commit comments

Comments
 (0)