GNOME Bugzilla – Bug 689528
switcherPopup: Factor out altTab and ctrlAltTab's common code
Last modified: 2012-12-04 09:13:20 UTC
Since I'm doing another switcher popup (for input sources) we might start sharing code right away.
Created attachment 230523 [details] [review] switcherPopup: Factor out altTab and ctrlAltTab's common code
Created attachment 230524 [details] [review] windowManager: Make use of the switch-panels-backward keybinding We have it in the schema so there's no reason for it not to work.
Created attachment 230525 [details] [review] AppSwitcher: Remove a lost timeout on destroy Doesn't look harmful but there's no point in running this code after we're destroyed.
(In reply to comment #1) > Created an attachment (id=230523) [details] [review] > switcherPopup: Factor out altTab and ctrlAltTab's common code Note that this is just code shuffling. There shouldn't be any logic changes here.
I'd want to wait for Florian to land his alt-alt-tab proposal first.
(In reply to comment #5) > I'd want to wait for Florian to land his alt-alt-tab proposal first. Nope, Rui and I agreed that it makes sense to land his changes first. I'm currently cleaning up an updated patch series on top of the patches here.
Review of attachment 230525 [details] [review]: Sure.
Review of attachment 230523 [details] [review]: You should be able to use Layout.MonitorConstraint here to remove even more code. Otherwise, fine. I can't spot any major flaws. ::: js/ui/windowManager.js @@ +622,3 @@ let modifiers = binding.get_modifiers(); let backwards = modifiers & Meta.VirtualModifier.SHIFT_MASK; + Main.ctrlAltTabManager.popup(backwards, binding.get_name(), binding.get_mask()); I think having a manager for both is a bit silly. I'd see if we can share a generic SwitcherManager, or if we even need the manager anymore.
Review of attachment 230524 [details] [review]: Sure.
Review of attachment 230523 [details] [review]: switcherPopup.js is missing from Makefile.am, causing it to not get installed, and the shell to crash on startup...
Thanks for the review. I'll think about further cleanups as I finish the input sources switcher. Pushing with the new file added to Makefile.am now. Attachment 230523 [details] pushed as 00338bb - switcherPopup: Factor out altTab and ctrlAltTab's common code Attachment 230524 [details] pushed as cd58f9f - windowManager: Make use of the switch-panels-backward keybinding Attachment 230525 [details] pushed as 5faeaa2 - AppSwitcher: Remove a lost timeout on destroy