Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: creates a new way to add shortcut key mappings #6122

Merged
merged 5 commits into from
May 13, 2022

Conversation

alschmiedt
Copy link
Contributor

@alschmiedt alschmiedt commented Apr 28, 2022

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

While I was writing a document talking about the different ways to customize Blockly I really wanted to say, "for the shortcut and context menu registries you simply create an object with all the necessary information and then register it." But that was not true for keyboard shortcuts because developers have to call addKeyMapping after registering their object. This was very annoying and since the fix is completely backwards compatible, this fixes that.

Proposed Changes

  • Add the ability to add a list of key codes to the shortcut object.
  • Updates shortcut items to use this instead of addKeyMapping.
  • Add the ability to allowCollisions from the shortcut object.

Behavior Before Change

In order to add a key code for a shortcut, developers would have to do:

ShortcutRegistry.registry.addKeyMapping('46', copyShortcut.name);

Behavior After Change

Now developers can do:

const copyShortcut = {
  ...
   keyCodes: ['46']
  ...
};

Reason for Changes

This is cleaner in most situations.

Note: This does not cover the keyboard navigation use case where developers will want to update and change key mappings based on user input. In that case they would still need to use addKeyMapping in that situation.

Additional Information

@alschmiedt alschmiedt requested a review from a team as a code owner April 28, 2022 00:10
@alschmiedt alschmiedt changed the title Update shortcut items feat: creates a new way to add shortcut key mappings Apr 28, 2022
Copy link
Collaborator

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

LGTM! Does this need docs changes as well? If so, could you file an issue for that?

@BeksOmega BeksOmega removed the request for review from NeilFraser May 5, 2022 17:23
@BeksOmega BeksOmega assigned BeksOmega and unassigned NeilFraser and maribethb May 5, 2022
@alschmiedt
Copy link
Contributor Author

Ok I added #6163 to add better documentation for shortcuts. I also updated the keyboard navigation codelab with this change. A PR for that is coming soon.

@alschmiedt alschmiedt merged commit adb5ad1 into google:develop May 13, 2022
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.

None yet

4 participants