Created attachment 285260 [details] [review]
switcherPopup: Ensure that we have items to show in a single place

This way all subclasses benefit from this sanity check.

Created attachment 285261 [details] [review]
switcherPopup: Factor the initial selection into the base class

Only the application switcher needs to keep its own implementation
since it has two modes of operation depending on the binding.

Created attachment 285262 [details] [review]
status/keyboard: Remove unused variable

Review of attachment 285260 [details] [review]:

LGTM

::: js/ui/switcherPopup.js
@@ +113,2 @@
     show: function(backward, binding, mask) {
+        if (!this._createSwitcher() || this._items.length == 0)

Would it make sense to swap the conditions? Or do we need to create the switcher in any case, even when we're not going to show it?

Review of attachment 285262 [details] [review]:

Sure

Review of attachment 285261 [details] [review]:

OK

Created attachment 285265 [details] [review]
switcherPopup: Move _createSwitcher implementations into constructors

We don't really need this step as a separate method since all
implementations are supposed to be created and shown immediately. This
also ensures that we have items to show in all subclasses.
--
(In reply to comment #4)

> Would it make sense to swap the conditions? Or do we need to create
> the switcher in any case, even when we're not going to show it?

Right. What about this patch instead?

Review of attachment 285265 [details] [review]:

OK

Attachment 285261 [details] pushed as 2b1077a - switcherPopup: Factor the initial selection into the base class
Attachment 285262 [details] pushed as 19e3f79 - status/keyboard: Remove unused variable
Attachment 285265 [details] pushed as 547cdf8 - switcherPopup: Move _createSwitcher implementations into constructors