GNOME Bugzilla – Bug 783550
windowManager: Add a switcher for mutter's switch-monitor keybinding
Last modified: 2017-08-22 16:46:24 UTC
The main patch here is more of a RFC for design feedback at this point, that's why I didn't split it to a new file yet.
Created attachment 353402 [details] [review] switcherPopup: Add support for modifier-less keybinding navigation This drops the requirement that SwitcherPopups need a modifier based keybinding to work. The existing behavior for modifier based keybindings is kept but if the popup is triggered from a no modifiers keybinding, instead of finishing when the modifier is released, we use a timer that automatically finishes the popup. The timer is reset on every key release to allow navigation to happen.
Created attachment 353403 [details] [review] windowManager: Add a switcher for mutter's switch-monitor keybinding
Created attachment 353463 [details] how it looks
This will need new icons. I can try to help here, but it would be better if one of our icon artists could take a look. There is some overlap with the icons used in the latest display settings here: https://wiki.gnome.org/Design/SystemSettings/Displays#Tentative_Guidelines I find the strings a little hard to parse and it would be good to try and make them as consistent as possible with the terminology that we have elsewhere. Perhaps something like: * Mirror * Join Displays * External Only * Built-in Only
Created attachment 357282 [details] [review] windowManager: Add a switcher for mutter's switch-monitor keybinding -- * moved into a new file * changed the labels as aday suggested and made them translatable * made it use the icons jimmac pushed to adwaita-icon-theme
Review of attachment 353402 [details] [review]: LGTM ::: js/ui/switcherPopup.js @@ +21,2 @@ const DISABLE_HOVER_TIMEOUT = 500; // milliseconds +const NO_MODS_TIMEOUT = 1500; // milliseconds Should probably be 'var' now as the constant above, in case some other module wants to reuse it ...
Review of attachment 357282 [details] [review]: ::: js/ui/switchMonitor.js @@ +15,3 @@ + + _init: function() { + let items = [ { icon: 'view-mirror-symbolic', Style nit - odd whitespace: [ { stuff }] Either put a space between bracket and braces or don't (the latter is predominantly used afaics)
As the strings have been changed (Mirror | Join Displays | External Only | Built-in Only) I assume rendering has been also tested with way longer strings (translations), maybe not having whitespaces (as some languages are funny)? Those strings might welcome translator comments that they refer to monitors (see https://wiki.gnome.org/TranslationProject/DevGuidelines/Use%20comments ) if you don't want noun translations of "mirror". (To be fair though, I don't see such translator comments in gnome-control-center either.)
(In reply to André Klapper from comment #8) > As the strings have been changed (Mirror | Join Displays | External Only | > Built-in Only) I assume rendering has been also tested with way longer > strings (translations), maybe not having whitespaces (as some languages are > funny)? > > Those strings might welcome translator comments that they refer to monitors > (see > https://wiki.gnome.org/TranslationProject/DevGuidelines/Use%20comments ) if > you don't want noun translations of "mirror". (To be fair though, I don't > see such translator comments in gnome-control-center either.) Added this kind of comment for each of them: /* Translators: this is for display mirroring i.e. cloning. * Try to keep it under around 15 characters. */
Got 2 release team acks: https://mail.gnome.org/archives/gnome-i18n/2017-August/msg00049.html https://mail.gnome.org/archives/gnome-i18n/2017-August/msg00061.html Attachment 353402 [details] pushed as c899453 - switcherPopup: Add support for modifier-less keybinding navigation Attachment 357282 [details] pushed as b35dfc8 - windowManager: Add a switcher for mutter's switch-monitor keybinding
*** Bug 600774 has been marked as a duplicate of this bug. ***