[core] fix(HotkeyParser): use e.code to get physical keys #6301
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adidahiya
commented
Jul 24, 2023
| export const SHIFT_KEYS: KeyMap = { | ||
| "~": "`", | ||
| "!": "1", |
There was a problem hiding this comment.
We no longer need the 0-9 digit keys in this mapping because we now handle events with { code: "DigitX" } in a more straightforward way. In a future PR, we can remove this KeyMap entirely by switching to event.code for more keys, but for now I decided to be conservative and make fewer behavioral changes in this PR.
Edit: I added a code comment to reflect this
[core] fix(HotkeyParser): use e.code to get physical keysBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
Fix self-review and lintBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
|
I think there's one more regression compared to v4.x which is worth fixing related to the "space" key. Currently with this PR:
On v4.x legacy docs:
|
use e.code for space keyBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6257
Checklist
Changes proposed in this pull request:
Refactor HotkeyParser functions to use
event.codeinstead ofevent.keyin most cases. This property is more reliable thanevent.keysince the latter is prone to changing based on keyboard layout and modifier keys. This change restores the behavior of the hotkey parser (which is responsible for taking a user-requested key combo and generating the appropriate event handler) to that of Blueprint v4.x (it regressed in v5.0 via #6106).I also updated the docs to move the "Key combo" and "hotkeys tester" sections to the non-deprecated
useHotkeysdocumentation (previously this was only available on the "Hotkeys (legacy)" docs page).Reviewers should focus on:
No regressions in
useHotkeys()Screenshot
before this change:
after this change:
in Blueprint v4.x legacy docs: