Created attachment 170031 [details] [review]
[appSwitcher] Close the switcher on Meta_L/R release

The keyReleaseEvent handler does not take into account that a user might
release Alt while still holding shift (the keycode because Meta_L/R in that case).

The result is that the switcher stays open which is unexpected.

Created attachment 170033 [details] [review]
[appSwitcher] Close the switcher on Meta_L/R release

*) Don't remove the blank line ...

Review of attachment 170031 [details] [review]:

The code looks good, just the commit message needs fixing:

the keycode because ... => the keycode becomes

(also that line does not fit in a standard terminal window, but maybe I'm just being anal here)

Attachment 170033 [details] pushed as 246d9f1 - [appSwitcher] Close the switcher on Meta_L/R release

This is not strictly correct; the fact that Shift+Alt gives Meta (and not some other modifier) is just a fact about your keyboard layout, not an innate truth.

What we really want to test is "is the Alt modifier no longer set after the keyrelease". You can't do that with Shell.get_event_state(event), because the event's state still reflects the alt key being down, but if you call Shell.get_pointer() to get a new modifier state, it should be correct (I think).

Created attachment 170038 [details] [review]
[appSwitcher] Check the modifier state rather than assuming Meta

It isn't guaranteed that Alt+Shift equals to Meta, so we have to check
the modifier instead.

Comment on attachment 170038 [details] [review]
[appSwitcher] Check the modifier state rather than assuming Meta

>+        if (keysym == Clutter.Alt_L || keysym == Clutter.Alt_R || state == 0)
>             this._finish();

don't even look at keysym. The dialog shouldn't go away if the user presses and releases the opposite alt key from the one they started with.

Created attachment 170039 [details] [review]
[appSwitcher] Check the modifier state rather than assuming Meta

It isn't guaranteed that Alt+Shift equals to Meta, so we have to check
the modifier instead.

Comment on attachment 170039 [details] [review]
[appSwitcher] Check the modifier state rather than assuming Meta

(assuming you tested that it works)

(In reply to comment #9)
> (From update of attachment 170039 [details] [review])
> (assuming you tested that it works)

I did ...

Attachment 170039 [details] pushed as 907dd43 - [appSwitcher] Check the modifier state rather than assuming Meta