Allow both cases in shift+hotkeys #67

Merged

escapee merged 1 commit from escapee/profectus-template:main into main

2024-02-22 00:18:21 +00:00

escapee commented

2024-02-21 19:21:05 +00:00

Collaborator

The inclusion of shift+ in the hotkey key makes it feel like keys such as shift+t should work, but as it stands such a hotkey would only function as shift+T. I believe this is undesirable, as T already inherently includes a shift, such a hotkey would feel like saying shift+shift+t.

This is largely fixed with a simple .toLowerCase() on the key, except for the fact that there are special keys that may be used alongside the shift key where there's no shift+ variant (e.g. shift+ArrowLeft). As such, I have gone for a backup hotkey check rather than simply lowercasing the key in the e.shiftKey case, which ideally should allow all possible shift+key combinations.

The inclusion of `shift+` in the hotkey key makes it feel like keys such as `shift+t` should work, but as it stands such a hotkey would only function as `shift+T`. I believe this is undesirable, as `T` already inherently includes a `shift`, such a hotkey would feel like saying `shift+shift+t`. This is largely fixed with a simple `.toLowerCase()` on the key, except for the fact that there are special keys that may be used alongside the shift key where there's no `shift+` variant (e.g. `shift+ArrowLeft`). As such, I have gone for a backup hotkey check rather than simply lowercasing the key in the `e.shiftKey` case, which ideally should allow all possible `shift+key` combinations.

escapee added 1 commit 2024-02-21 19:21:06 +00:00

Allow both cases in shift+hotkeys

Run Tests / test (pull_request) Successful in 2m0s

Details

b204e3cc42

escapee force-pushed main from b204e3cc42 to b40d4bef32

2024-02-21 19:21:21 +00:00

Compare

thepaperpilot approved these changes 2024-02-21 23:48:00 +00:00

thepaperpilot left a comment

Collaborator

Good change. I was going to note that it doesn't handle numbers turning into symbols, but I don't think those are consistent across keyboard layouts anyways. LGTM!

escapee commented

2024-02-22 00:16:42 +00:00

Author

Collaborator

Those should also be handled already by the "uppercase numbers" code that you have, although it might be good to make sure we support direct symbol use in hotkey keys too (in another PR, if those aren't already working)