Implement keyboard localization support for player key commands #693
Conversation
The changes enable consistent keyboard shortcuts for media player commands across different keyboard layouts irregardless of the localization. This is achieved by identifying commands based on the character output of keys instead of their key codes which may vary. Due to the non-textual nature of the arrow keys their input remains to be identified by their keycodes.
There was a problem hiding this comment.
Overall this is a great improvement! If you have time could you consider the adjustments I suggested?
If you don't have the time, please let us know, we could probably take this as is as both of the critical suggestions I made also apply to the existing code!
| default: break | ||
| } | ||
|
|
||
| guard let character = event.charactersIgnoringModifiers else { |
There was a problem hiding this comment.
Reading through the docs, keyCode and charactersIgnoringModifiers both say they they can raise exceptions if the event is not a key event.
addLocalMonitorForEvents(matching: .keyDown) should ensure it's always a key event, but this function could use a guard for the type of event for thoroughness.
To increase the accuracy of this function, we could also ensure no modifiers are pressed as well via event.modifierFlags.isEmpty.
| @@ -1165,8 +1187,8 @@ public final class PUIPlayerView: NSView { | |||
|
|
|||
| keyDownEventMonitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { [unowned self] event in | |||
| guard self.isEnabled else { return event } | |||
|
|
|||
| guard let command = KeyCommands(rawValue: event.keyCode) else { | |||
|
|
|||
There was a problem hiding this comment.
nit: trailing space on empty line
|
Thanks @boettges! @allenhumphreys I'll apply your suggestions and submit another PR with just them. |
|
@allenhumphreys Those are very valid suggestions. Thank you both! |
The changes enable consistent keyboard shortcuts for media player commands across different keyboard layouts irregardless of the localization. This is achieved by identifying commands based on the character output of keys instead of their key codes which may vary. Due to the non-textual nature of the arrow keys their input remains to be identified by their keycodes.
Example:
The current mapping for
minusandplusis set to the keycodes27and24.This is not applicable to all keyboard layouts. For example, German localized ones use
44and30respectively.The proposed changes should solve this.
Thank you in advanced for reviewing and thank you for the handy app 👍