Some thoughts regarding KeyboardShortcut component #385
Comments
|
about this
The component currently offers a means for callers to provide translations (see As for the order, and leaving shift first, I did not know that. And I don't think we acknowledge that convention in either Twist or Todoist, even before the introduction of this component. So it was not taken into consideration when designing it. The order was something left to the caller to provide, with the assumption that the same order could apply to all platforms. About your proposal:
How would this help? I don't want apps to have to repeat the logic of Shift having to go first when on MacOS. That would make the calls to this component too verbose.
This was inherited from what Twist has always done before this component was introduced. It reflects the fact that we often use Cmd in macOS as we use Ctrl in other platforms. What would you do with a keyboard shortcut specified with Cmd in it, if this key does not exist in other platforms? All in all, yes, I agree that it could use a revamp better knowing its design goals. |
|
Thank you. I think translation approach makes sense, this should be enough (whether we need to translate something needs more research).
Do you think component should handle this? E.g. we could have ordering callback, similarly to translate callback?
I think this decision was guided by in-the-browser-Web platform. CMD key is 91 code, which is WIN key on Windows (I am unsure about practical usability of WIN key). |
Translation is different because each app has its own translation infrastructure so the lib cannot assume anything about it. But I'd say that with ordering it's not the same. If we know it is a hard rule that on macOS the shift key has to go first, but in Windows and Linux the opposite is true, then if the lib is able to absorve this complexity, it should. And I think it is perfectly able. It does not need to know something from the app, such as how it translates stuff.
This component does not consider key codes. It handles conceptual key names only, from how the user perceives these keys to represent. So this key thing is irrelevant. The practical thing that this conversion is achieving is that a big chunk of the time (if not always) when a custom keyboard shortcut in our apps is Can you illustrate with an example a situation where having the component follow this logic would result in a problem? |
I don't see how Cmd has anything to do with Ctrl, I think the only reason for this mapping is that it was lifted from Twist codebase. The fact that Ctrl+F and Cmd+F are distinct on macOS but same (?) on Windows is just confusing IMO. I need to talk to someone on Windows to better understand it.
Not sure if I understand what you mean here. I do realize that the whole 'keyCode' vs 'key' in JavaScript issue is very problematic. But I would say that this component only deals with display, so it's not relevant to it (e.g. on my keyboard RightOption+A is ą, but I would still expect it to be shown as |
|
Valid points. Indeed, this was not made with windows shortcuts in mind, and was made with some assumptions on our approach to shortcuts, that now with hybrid native-like apps in the horizon may need revisiting. This component does have a mechanism to express the default platform-specific modifier, and it is via the "mod" key name. It interprets it as Cmd in macOS. We can enhance it so that this special key name is also interpreted as the Another modification that it may need is that it probably should support apps specifying different shortcuts for different platforms Not sure how complex this can get without making the calls to this component not too complex. I'll think how this component's interface can be better defined for a more versatile use cross platforms without compromising the ease with which it's called. I'd hate for it too become a behemoth with too many props (e.g. a prop for the shortcut in macos, plus a prop for the shortcut in Windows, etc.) |
I don't think this is the way to go. Shortcuts could just as well be dynamic (configurable by the user), could change by browser (to avoid clashes) etc. I think it's much easier to replace shortcuts on client level (e.g. the way we do here by replacing the whole keymap or tweaking some of the constants). |
|
Proper ordering of macOS modifier keys is (take from Apple Style Guide https://books.apple.com/us/book/apple-style-guide/id1161855204): Fn (function), Control, Option, Shift, Command |
|
@proxi I can't find a similar guideline on the order for Windows, but e.g. look at some official shortcuts here: A common pattern is for Shift to come second: In general, most shortcuts only use 2 modifiers although sometimes, three appear too
Windows key always goes first.
|
🐛 Bug report
Current behavior
KeyboardShortcut component seems to make multiple assumptions that seem specific to usage in our codebases, to web, to English locale, and more. In particular, it remaps CMD to CTRL on platforms other than macOS.
The problem is that this is not enough to meet platform conventions. On macOS, SHIFT should be first, and only replacing characters without reordering them does not result in display following platform guidelines.
CMD + SHIFT + 1renders asCtrl Shift 1on Windows and⌘ ⇧ 1on macOS, but it should be⇧ ⌘ 1on macOS.Other potential issues:
Possible solutions
The component could only do rendering, and leave all other decisions to the caller.
The text was updated successfully, but these errors were encountered: