Skip to content

Commit 53e8c61

Browse files
authored
fix(blocks): release isBlockMoving when a drag ends any other way (#8363)
moveBlockRelative and moveBlockRelativeBatched raise blocks.isBlockMoving, and block.js calls moveBlockRelativeBatched on every pressmove before any threshold is applied. The only place that lowered the flag was the tail of blockMoved(), which block.js reaches only when a drag passed the 5px movement threshold, ended, and finished away from the trashcan. Two everyday actions therefore latched the flag on for the rest of the session. Pressing a block and moving less than 5px runs the move but leaves `moved` false, so the whole branch that would call blockMoved is skipped. Dragging a stack onto the trashcan runs sendStackToTrash instead, which never touches the flag. blockMoved's own two guard clauses returned early past the clear as well. selection-controller gates the entire drag-select animation frame on !isBlockMoving, and drawSelectionArea, selectBlocksInDragArea and setSelectedBlocks are reachable from nowhere else, so rubber-band selection silently stopped working with no error and no visible cause. It only recovered if the user happened to perform a normal drag that ran blockMoved to completion. Lower the flag in clearCachedDragGroup() instead. Both the mouseout and pressup handlers call that unconditionally once a drag is over, whichever way it ended, so it covers the trashcan and sub-threshold paths together. The two guard clauses in blockMoved also clear it, since blockMoved is additionally reachable from the keyboard controller and the palette, which do not run the drag teardown. Four of the seven added tests fail without this change. The other three assert that the flag is still raised on a move and still lowered by a completed blockMoved, so the behaviour cannot be "fixed" by simply dropping the flag. Signed-off-by: netram75 <netram.24bcs10329@sst.scaler.com>
1 parent dbaf3bc commit 53e8c61

2 files changed

Lines changed: 91 additions & 0 deletions

File tree

js/activity/__tests__/block-drag-controller.test.js

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,4 +1427,86 @@ describe("BlockDragController", () => {
14271427
expect(blocks._cachedDragGroup).toEqual([1]);
14281428
});
14291429
});
1430+
1431+
// ------------------------------------------------------------------
1432+
// isBlockMoving lifecycle
1433+
// ------------------------------------------------------------------
1434+
1435+
describe("isBlockMoving lifecycle", () => {
1436+
const oneBlock = () => [
1437+
makeFlowBlock({ x: 0, y: 0, docks: [[0, 0, "in"]], connections: [null] })
1438+
];
1439+
1440+
it("is raised by moveBlockRelativeBatched", () => {
1441+
const blocks = makeBlocks(oneBlock());
1442+
1443+
expect(blocks.isBlockMoving).toBe(false);
1444+
blocks.moveBlockRelativeBatched(0, 5, 5);
1445+
1446+
expect(blocks.isBlockMoving).toBe(true);
1447+
});
1448+
1449+
it("is raised by moveBlockRelative", () => {
1450+
const blocks = makeBlocks(oneBlock());
1451+
1452+
blocks.moveBlockRelative(0, 5, 5);
1453+
1454+
expect(blocks.isBlockMoving).toBe(true);
1455+
});
1456+
1457+
it("is lowered by a completed blockMoved", async () => {
1458+
const blocks = makeBlocks(oneBlock());
1459+
blocks.moveBlockRelativeBatched(0, 5, 5);
1460+
1461+
await blocks.blockMoved(0);
1462+
1463+
expect(blocks.isBlockMoving).toBe(false);
1464+
});
1465+
1466+
// The drag teardown path. block.js calls clearCachedDragGroup() from
1467+
// both the mouseout and pressup handlers once a drag is over, whether
1468+
// or not blockMoved ran, so a drag that ended on the trashcan or that
1469+
// never passed the 5px movement threshold is released here.
1470+
it("is lowered by the drag teardown even when blockMoved never runs", () => {
1471+
const blocks = makeBlocks(oneBlock());
1472+
blocks.moveBlockRelativeBatched(0, 2, 1);
1473+
expect(blocks.isBlockMoving).toBe(true);
1474+
1475+
blocks.clearCachedDragGroup();
1476+
1477+
expect(blocks.isBlockMoving).toBe(false);
1478+
});
1479+
1480+
it("is lowered when blockMoved bails out on a null block", async () => {
1481+
const blocks = makeBlocks(oneBlock());
1482+
blocks.moveBlockRelativeBatched(0, 5, 5);
1483+
1484+
await blocks.blockMoved(null);
1485+
1486+
expect(blocks.isBlockMoving).toBe(false);
1487+
});
1488+
1489+
it("is lowered when blockMoved bails out on a missing block", async () => {
1490+
const blockList = oneBlock();
1491+
const blocks = makeBlocks(blockList);
1492+
blocks.moveBlockRelativeBatched(0, 5, 5);
1493+
1494+
// An index that is not present in blockList reaches the second
1495+
// guard, after the expandable scan has already run.
1496+
await blocks.blockMoved(99);
1497+
1498+
expect(blocks.isBlockMoving).toBe(false);
1499+
});
1500+
1501+
it("leaves the rest of the cached drag state cleared too", () => {
1502+
const blocks = makeBlocks(oneBlock());
1503+
blocks.moveBlockRelativeBatched(0, 5, 5);
1504+
1505+
blocks.clearCachedDragGroup();
1506+
1507+
expect(blocks._cachedDragGroup).toBeNull();
1508+
expect(blocks._dragActiveGroup).toBeNull();
1509+
expect(blocks.isBlockMoving).toBe(false);
1510+
});
1511+
});
14301512
});

js/activity/block-drag-controller.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ class BlockDragController {
100100
const blocks = this.blocks;
101101
blocks._cachedDragGroup = null;
102102
blocks._dragActiveGroup = null;
103+
// moveBlockRelative/moveBlockRelativeBatched raise isBlockMoving on
104+
// every pressmove, but blockMoved() is the only place that lowers it
105+
// and it runs only for a drag that passed the movement threshold and
106+
// ended away from the trashcan. Clearing here instead covers every
107+
// way a drag can finish, since both the mouseout and pressup handlers
108+
// call this unconditionally once the drag is over.
109+
blocks.isBlockMoving = false;
103110
}
104111

105112
/**
@@ -258,6 +265,7 @@ class BlockDragController {
258265
blocks.clampBlocksToCheck = [];
259266
if (thisBlock === null) {
260267
console.debug("blockMoved called with null block.");
268+
blocks.isBlockMoving = false;
261269
return;
262270
}
263271

@@ -292,6 +300,7 @@ class BlockDragController {
292300
const myBlock = blocks.blockList[thisBlock];
293301
if (myBlock === null || myBlock === undefined) {
294302
console.debug("null block found in blockMoved method: " + thisBlock);
303+
blocks.isBlockMoving = false;
295304
return;
296305
}
297306

0 commit comments

Comments
 (0)