-
Notifications
You must be signed in to change notification settings - Fork 112
Use KeyboardEvent#code in camera-fly.ts #1460
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
base: main
Are you sure you want to change the base?
Conversation
List of all PlayCanvas editor hotkeys
|
TestingPotentially affecting all hotkeys containing the letters ZQSDAE
From local testing
All the ones with editor/src/editor/viewport/camera/camera-fly.ts Lines 74 to 76 in a3f4963
|
|
So it seems like the Shift + Z shortcut has an almost decade long history: https://github.com/playcanvas/editor-ui/commit/c5b1cede34324b79c4d97d8d123db951407a9ecc and is also documented in various places (Help popup and PC Dev Docs) So I see the following options: Option 1: Keep the Shift + Z shortcut
Option 2: Rebind the shortcut Shift + Z to something else
Option 3: Do not merge this change in
Of course long term we should have custom keybindings that have been long requested (e.g. #344 and #408 ) to avoid such conflicts in the first place. This PR is a short term solution to give AZERTY users a better experience in the meanwhile. |
…conflict with AZERTY orbital movement
|
Chose for Option 2 and used Shift + X:
|
|
Why not use code for shift+Z also, so it keeps working in same position as in english layout and no conflict will be there? Basically if |
So I cancelled this PR #1461 because from researching a little bit further, users using other layouts don't use the physical keys in most applications. So for a German user, their muscle memory would be to press Ctrl + Z ( The problem with having physical keys in the shortcut is that it's also hard to communicate with new users what keys they would have to press. Having custom keybindings would solve all of this, but that requires more effort in the future. |
kpal81xd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Will be a breaking change to make sure to include when doing release notes

Fixes #1451 specifically
What has changed?
Using
KeyboardEvent#code(MDN ) for orbit control, so that the physical keys get recognised instead of their valuesGood tool to check the different keyboard codes: https://www.toptal.com/developers/keycode
I confirm I have read the contributing guidelines