Skip to content

Conversation

@BenHenning
Copy link
Collaborator

@BenHenning BenHenning commented May 15, 2025

Fixes #525

These tests were broken as a result of RaspberryPiFoundation/blockly#9045 and have been updated as such:

  • Many of these existing getCurrentFocusNodeId calls have been updated to use a new getCurrentFocusedBlockId since they actually want to match against a block's data ID rather than its focus-specific ID (which is new as of fix: Ensure BlockSvg is uniquely focusable on the page blockly#9045).
  • There are still some existing uses for getCurrentFocusNodeId (specifically fields).

Separately, clickBlock has been removed. Its uses were removed in #482 (I think) in favor of direct keyboard navigation. I suspect this is the pattern we want to keep moving forward since it has better alignment with real user behavior, so removing the helper ensures tests don't fall back on it instead of trying to use real navigation.

Finally, this PR also updates the plugin to 12.0.0 of Blockly now that it's fully launched. I've tested this against a variety of scenarios yesterday and today so it seems to be working pretty well (#526 notwithstanding). Additionally, the CI workflows have been updated to link against latest develop as the new "tip-of-tree" v12. Unfortunately there was a behavioral discrepancy between beta.7 and the full 12.0.0 release which resulted in 1 new test failing. This will be fixed by RaspberryPiFoundation/blockly#9063 but it means two things:

  1. The CI for this PR won't pass until fix: Ensure FieldImage is clickable when appropriate blockly#9063 is merged.
  2. The main branch's CI won't pass once this is merged due to that one test failing. Once we push a 12.0.1 or 12.1.0 version of Blockly and update this plugin accordingly, the test will start passing in main's CI.

These correctly link against develop for v12 tip-of-tree, instead. This
also includes some renaming in/of the workflows for clarity.
@BenHenning
Copy link
Collaborator Author

Currently investigating 1 new failure that is somehow different between beta.7 and 12.0.0.

@BenHenning
Copy link
Collaborator Author

BenHenning commented May 15, 2025

Ah. From testing it seems that RaspberryPiFoundation/blockly#9054 caused this new failure.

Test Right from text block selects shadow block then field is failing due to the quote image fields now becoming navigable.

Using 'v12' is more generic and will automatically be correct when the
plugin moves to an eventual 12.0.1 or 12.1.0.
@BenHenning
Copy link
Collaborator Author

I've verified that the tests are passing when linked against RaspberryPiFoundation/blockly#9063. This PR should block on being merged until its CI passes (i.e. after RaspberryPiFoundation/blockly#9063 is merged), however it can still probably be reviewed now.

Copy link
Collaborator Author

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Self-reviewed changes.

FYI I noticed that my regeneration of package-lock.json includes some possibly unexpected removals. While everything is running well locally, I wonder if this has something to do with my local npm version.

@BenHenning BenHenning marked this pull request as ready for review May 15, 2025 23:29
@BenHenning BenHenning requested a review from a team as a code owner May 15, 2025 23:29
@BenHenning BenHenning requested review from maribethb and removed request for a team May 15, 2025 23:29
@rachel-fenichel rachel-fenichel mentioned this pull request May 16, 2025
1 task
@rachel-fenichel rachel-fenichel merged commit a0bf76f into RaspberryPiFoundation:main May 16, 2025
9 of 11 checks passed
@BenHenning BenHenning deleted the fix-tests-and-upgrade-to-v12 branch May 19, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests are broken due to ID changes

3 participants