Skip to content

Conversation

@maribethb
Copy link
Collaborator

@maribethb maribethb commented Jul 31, 2025

Fixes #675

This ends up being a somewhat random collection of things but they were all needed to set up the tests + get them to pass.

Test setup / config related stuff:

  • Adds non-webdriver mocha tests.
    • You can add more by adding new test files that end in .mocha.js.
    • These are cjs files, so they're not compatible with chore: Add type: "module" to package.json #611 without that PR doing additional work (which might need to happen in blockly-samples because the blockly-scripts test script is specifically looking for .mocha.js files not .mocha.cjs files)
    • These run automatically when running npm run test or you can run them alone with npm run test:mocha
    • One downside of this is that if these tests fail, the webdriver tests won't run at all. So you can't see all the test failures at once. I'm sure there's a way around this but I got the impression from Ben we couldn't use gulp to run the tests and that's how we manage the multiple tests in core, so idk. open to suggestions.
  • Fixes a simple problem with the test command that was causing the webdriver tests to be built twice.
  • Fixes a problem with some files importing from src/foo instead of ../foo

Actual plugin code stuff:

  • clipboard shortcuts now fully clean up after themselves
  • the commitMove shortcut is registered when a move starts and unregistered when a move finishes. now also unregister it (conditionally) in case the workspace was possibly disposed during a move
  • the move shortcuts did not clean up after themselves properly as the array simply grew in size with every call to install as it was never reset. rearranged the code so the array is only built once.
    • unfortunately the diff for this is horrible, sorry. if you select "hide whitespace" in the github review options, it looks much more reasonable.
    • I didn't change the actual shortcut items at all.
    • I only changed the displayText for the context menu items to use an arrow function instead of calling getMenuItem directly, since the function would now be evaluated earlier. otherwise the code is unchanged.
  • allowed collisions on the edit_or_confirm shortcut, and made its precondition return false if the workspace is dragging, since in that case the finishMove shortcut is what will happen for the Enter key. this makes the shortcuts more resilient to being installed in different orders since their preconditions now should never overlap.

@maribethb maribethb requested a review from a team as a code owner July 31, 2025 01:08
@maribethb maribethb requested review from RoboErikG and removed request for a team July 31, 2025 01:08
@maribethb
Copy link
Collaborator Author

Since it's buried in the middle of my essay, just want to call your attention to the fact that the diff looks a lot better if you select the "hide whitespace" option in the github review UI, especially in the actions/move file.

Copy link
Contributor

@RoboErikG RoboErikG left a comment

Choose a reason for hiding this comment

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

LGTM. One minor suggestion, but feel free to ignore.

@maribethb maribethb merged commit 3696e2b into RaspberryPiFoundation:main Aug 1, 2025
8 checks passed
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.

Dispose + reinitialize keyboard navigation issues

2 participants