Skip to content

More Scratch Blocks fixes#482

Merged
cwillisf merged 2 commits into
developfrom
more-scratch-blocks-fixes
Mar 17, 2026
Merged

More Scratch Blocks fixes#482
cwillisf merged 2 commits into
developfrom
more-scratch-blocks-fixes

Conversation

@cwillisf
Copy link
Copy Markdown
Contributor

Resolves

Proposed Changes

  • Don't ignore keypresses if a Blockly "content node" is focused
  • Disable Blockly events, rather than removing and re-adding the event listener, while switching blocks workspaces due to a sprite switch

Reason for Changes

  • The VM should handle keys relayed from Blockly even if an SVG object is selected, but not if focus is in an input element. As we move toward full keyboard nav support, we may need Blockly / Scratch Blocks to be more careful about when it reports key events to the VM, but the coarse filtering here was rejecting important events.
  • Newer Blockly uses async events, so temporarily removing and later re-adding the event handler didn't guarantee suppression of events that were generated before the handler was re-added. Blockly now has an explicit call to disable and re-enable generating events, so use that.

Test Coverage

Tested locally with the "Eat the Pi!" project, which is sensitive to both of these issues.

@cwillisf cwillisf requested a review from Copilot March 17, 2026 15:58
@cwillisf cwillisf requested a review from a team as a code owner March 17, 2026 15:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Scratch Blocks–related regressions affecting keyboard input routing and workspace switching, ensuring the VM continues to receive intended key events and that workspace reloads don’t emit spurious Blockly events.

Changes:

  • Adjust keyboard event filtering so SVG-targeted (Blockly workspace) keydowns are forwarded to the VM even when a block has Blockly focus.
  • Update unit test expectations for VM keyboard event forwarding from the Blockly SVG workspace.
  • During workspace reload on sprite switch, disable/enable Blockly event generation instead of removing/re-adding the VM block listener.

Reviewed changes

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

File Description
packages/scratch-gui/test/unit/util/vm-listener-hoc.test.jsx Updates unit test to assert SVG (Blockly workspace) keydowns are forwarded to the VM.
packages/scratch-gui/src/lib/vm-listener-hoc.jsx Removes ScratchBlocks-focused filtering; forwards SVG-targeted keydowns to VM while still ignoring HTML input targets.
packages/scratch-gui/src/containers/blocks.jsx Switches from listener removal to ScratchBlocks.Events.disable()/enable() during workspace reload to avoid async-queued event leakage.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment thread packages/scratch-gui/src/containers/blocks.jsx Outdated
Comment thread packages/scratch-gui/src/containers/blocks.jsx Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 17, 2026

Test report for scratch-gui

  2 files   63 suites   10m 14s ⏱️
400 tests 392 ✅ 8 💤 0 ❌
418 runs  410 ✅ 8 💤 0 ❌

Results for commit 08016f80.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes two Scratch Blocks / Blockly integration issues in Scratch GUI: (1) keyboard events from the Blockly SVG workspace were being overly filtered and not reaching the VM in some focus states, and (2) switching workspaces during sprite changes could leak queued async Blockly events to the VM.

Changes:

  • Adjust VM keyboard event forwarding to always allow SVG-targeted (Blockly workspace) keydown events through to the VM.
  • Switch workspace reload suppression from temporarily removing listeners to disabling/enabling Blockly events around workspace reload.
  • Add unit coverage for ensuring ScratchBlocks.Events.enable() is always called after a workspace reload attempt (success or failure).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/scratch-gui/test/unit/util/vm-listener-hoc.test.jsx Updates keyboard forwarding expectations to match the new SVG-focused behavior.
packages/scratch-gui/test/unit/containers/blocks.test.js Adds unit tests ensuring Blockly events are re-enabled even if workspace load/parsing throws.
packages/scratch-gui/src/lib/vm-listener-hoc.jsx Removes isContentNodeFocused gating so SVG-targeted keydown events always reach the VM.
packages/scratch-gui/src/containers/blocks.jsx Disables/enables Blockly events during workspace reload and exports Blocks for unit testing.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment thread packages/scratch-gui/src/lib/vm-listener-hoc.jsx
@cwillisf cwillisf merged commit 4271431 into develop Mar 17, 2026
15 checks passed
@cwillisf cwillisf deleted the more-scratch-blocks-fixes branch March 17, 2026 17:54
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switching sprites can break a running project

2 participants