Fix/trash touch delete

[soumyalimje]

Summary

Fixes a race on touch devices where dragging a block into the trash, waiting until the trash highlight turns red, and releasing could fail to delete the block because the highlight completed slightly after the release event.
Instead of immediately deleting when the highlight completes, the trash sets a small confirmation state (a trashReady flag recording the active block). The block's existing release handler now checks that flag and deletes on release. This preserves confirm-on-release UX while solving the timing race for touch users.

[github-actions]

✅ All Jest tests passed! This PR is ready to merge.

[Copilot]

Pull request overview

This pull request addresses a race condition on touch devices where dragging a block to the trash and releasing could fail to delete the block if the trash highlight animation completed slightly after the release event. The fix introduces a trashReady flag mechanism that marks when the trash highlight completes, allowing the block's release handler to check this flag and proceed with deletion even if the timing was slightly off.

Changes:

• Added trashReady and trashReadyBlock flags to track when trash highlight animation completes and which block it applies to
• Modified block release logic to check trash ready flags in addition to trash visibility
• Added a new getBlockAtCoordinate helper function (appears unused)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
js/trash.js	Sets trashReady flags when highlight completes; clears flags when highlight stops
js/blocks.js	Initializes trashReady flags and adds getBlockAtCoordinate helper function
js/block.js	Updates deletion logic to check trashReady flags during block release

Comments suppressed due to low confidence (1)

js/block.js:3218

• The trashReady flags are not cleared when the block is released outside the trash area. If trashReady is set but the user drags the block away from the trash and releases it elsewhere, the flags remain set until stopHighlightAnimation is called. This could lead to unexpected behavior where a subsequent block operation might incorrectly think the trash is ready. Consider adding flag clearing logic in the else branch at line 3208 when the block is moved but not over the trash.

            } else {
                // Otherwise, process move.
                // Also, keep track of the time of the last move.
                this.blocks.mouseDownTime = new Date().getTime();
                this.blocks.blockMoved(thisBlock);

                // Just in case the blocks are not properly docked after
                // the move (workaround for issue #38 -- Blocks fly
                // apart). Still need to get to the root cause.
                this.blocks.adjustDocks(this.blocks.blockList.indexOf(this), true);
            }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The condition has a logical flaw. If trashReady is true for blockA, but blockB is released over the trash, blockB would be deleted because line 3193 only checks trashReady without verifying which block it applies to. The condition should be restructured to ensure that when using the trashReady mechanism, we verify it's the correct block. Consider changing the logic to: (this.activity.trashcan.isVisible || (this.blocks.trashReady && this.blocks.trashReadyBlock === thisBlock))

Suggested change

	this.blocks.trashReady ||
	this.blocks.trashReadyBlock === thisBlock
	(this.blocks.trashReady &&
	this.blocks.trashReadyBlock === thisBlock)

[Copilot]

The indentation of the trashReady and trashReadyBlock property declarations is inconsistent with the surrounding code. These lines should be indented with 8 spaces to match the indentation pattern of other properties in this constructor (see lines 236, 238, 240, and 255).

Suggested change

	this.trashReady = false;
	this.trashReadyBlock = null;
	this.trashReady = false;
	this.trashReadyBlock = null;

[Copilot]

The getBlockAtCoordinate function is defined but appears to be unused in the codebase. Consider removing this function if it's not needed for the fix, or if it was added for future use, ensure it's actually called somewhere.

Suggested change

	/**
	* Returns the index of the block at the given coordinates, or -1 if none.
	* @param {number} x - x coordinate
	* @param {number} y - y coordinate
	* @returns {number}
	*/
	this.getBlockAtCoordinate = function (x, y) {
	for (let i = 0; i < this.blockList.length; i++) {
	const block = this.blockList[i];
	if (!block || block.trash) continue;
	const blockX = block.container.x;
	const blockY = block.container.y;
	if (
	x >= blockX &&
	x <= blockX + block.width &&
	y >= blockY &&
	y <= blockY + block.height
	) {
	return i;
	}
	}
	return -1;
	};