- Issue created by @justcaldwell
- πΊπΈUnited States justcaldwell Austin, Texas
Small update to the MR. I realized we should prevent transmission of the keypress for Alt + p as well, in case the cursor is somewhere that can receive keyboard input.
- πΊπΈUnited States justcaldwell Austin, Texas
Adding static patch for testing.
- π«π·France dydave
Thanks a lot Michael (@justcaldwell), that's very helpful!
We didn't really carry any real testing with MacOS when working on these keyboard shortcuts and since we knew there would be issues, we added a configuration option to disable them, if needed, for the time being.
There is already a follow-up issue for this scope of work, see β¨ Allow shortcut key to be configurable Active :
which aims at making these shortcuts more flexible, with the ability to select different key combinations, see some elements of discussion in the following issue:
#3494646-6: Access Admin Toolbar search field via keyboard shortcut Alt + a βAllowing users to configure shortcut, either through the interface or at the very least with some kind of override in code.
The idea behind this feature would be to take a more generic approach to solving any issues with conflicting keyboard key combinations (whatever language, system, OS, etc...).Is this something we could trying moving to the Feature request instead and group our development efforts?
Otherwise, concerning the merge request and the use of
event.code === 'KeyA'
instead ofevent.key === 'a'
:We already tried this implementation and encountered the following issue #3494646-5: Access Admin Toolbar search field via keyboard shortcut Alt + a β :
4 - But most important: Changed the condition
event.code === 'KeyA'
toevent.key === 'a'
.
When I tested the initial code, the feature didn't work with my keyboard (French azerty) π
Turns out on my keyboard the KeyA corresponds to the key Q π (Q <==> A on French keyboards, the key positions are switched)So it felt like the
event.code
property couldn't be reliable enough and a different property should be used instead.
Printing theevent
object allowed me to check which other properties or keys could be used and I foundevent.key: 'a'
.Good thing we have different types of keyboards, regional settings, languages, etc... π
It certainly allows us to cover more potential use cases πSo it definitely seems there is more complexity to try matching a specific keyboard combination.
Therefore, we thought we should keep it simple in this first version and try to extend this feature in priority, in the next version.The current shortcuts are going to work fine in many cases (default) and probably not in other ones with conflicts for various reasons (browser plugin shortcut, language, keyboard type, etc....), but as long as they could be disabled, for the time being, it should not be a problem with the module.
Until we could β¨ Allow shortcut key to be configurable Active , so that a wider range of specific keyboard setups or combinations could be supported.Any feedback, comments or advice would certainly be greatly appreciated.
Thanks in advance!