Skip to content

Conversation

@maribethb
Copy link
Contributor

The basics

  • I branched from goog_module
  • My pull request is against goog_module
  • My code follows the style guide
  • My code is presented in the form suggested in the module
    conversion guide
  • I have run npm test.

The details

Resolves

Part of #5026 and #5208

Proposed Changes

Moves Blockly.onKeyDown to a local method on inject.js. This method was @package and only called from Blockly.inject to handle all keydown events on the page. This probably isn't something external developers would have been calling since we register it for them (and they shouldn't be since it's package) so I did not create an alias for it. Also, this is not marked as a breaking change since this should not affect code outside of Blockly.

Since the function is now not exported from its module, it is not accessible outside of this file. For the unit tests, I changed the tests to dispatch an event on the document instead of calling the event handler directly.

Additional Information

In order to get the unit tests to work with dispatchEvent, I had to create an actual KeyboardEvent object instead of the fake implementation previously used in the test_helpers. You can't just change the target type in the real event like this, so I removed that part. The only time it was ever used was for the "Not called when focus is an HTML input test" but that test was actually already broken. The event was created by passing in this.event as the first argument, which should be a keycode. So the keycode was not ESC so hideChaff was not called for that reason, and not because of the input. I've fixed this by dispatching the event on an actual html input, which correctly does not call the keyboard shortcut.

I also had to change the shortcut_registry tests to use an actual keycode instead of a random string, otherwise the event would not be created correctly.

@maribethb maribethb requested a review from a team as a code owner September 16, 2021 03:33
@maribethb maribethb requested a review from gonfunko September 16, 2021 03:33
@google-cla google-cla bot added the cla: yes Used by Google's CLA checker. label Sep 16, 2021
@github-actions github-actions bot added this to the 2021_q3_release milestone Sep 16, 2021
@maribethb maribethb changed the title refactor: Migrate Blockly.svgResize and Blockly.onKeyDown to Blockly.common refactor: Migrate Blockly.onKeyDown out of blockly.js Sep 16, 2021
@maribethb maribethb merged commit 1334bd3 into RaspberryPiFoundation:goog_module Sep 16, 2021
@maribethb maribethb deleted the module_onkeydown branch September 16, 2021 21:48
@maribethb maribethb mentioned this pull request Sep 21, 2021
33 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Used by Google's CLA checker. type: cleanup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants