Merge lp://qastaging/~3v1n0/unity/super+tab-shortcut-fixes into lp://qastaging/unity

Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 2044
Proposed branch: lp://qastaging/~3v1n0/unity/super+tab-shortcut-fixes
Merge into: lp://qastaging/unity
Prerequisite: lp://qastaging/~thomir-deactivatedaccount/unity/fix-ap-test-stability
Diff against target: 510 lines (+304/-101)
6 files modified
manual-tests/SuperTab.txt (+1/-20)
plugins/unityshell/src/Launcher.cpp (+2/-1)
plugins/unityshell/src/LauncherController.cpp (+2/-2)
plugins/unityshell/src/unityshell.cpp (+6/-0)
tests/autopilot/autopilot/emulators/unity/launcher.py (+8/-0)
tests/autopilot/autopilot/tests/test_launcher.py (+285/-78)
To merge this branch: bzr merge lp://qastaging/~3v1n0/unity/super+tab-shortcut-fixes
Reviewer Review Type Date Requested Status
Thomi Richards (community) Approve
Review via email: mp+95288@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2012-02-01.

Description of the change

The launcher icons can be shown during the Super Tab Launcher switcher.

For both the shortcut hints and the icons shortcut overlays we use the same policy:
 - If they were shown before starting the Super+Tab session, we still show them
 - If they were not shown before starting the Super+Tab session, they won't be ever shown

Plus, if we're pressing a valid unity shortcut key when the Launcher Switcher is active, we should terminate the switcher without doing anything.

UNBLOCK

To post a comment you must log in.
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote : Posted in a previous version of this proposal

Hi Marco,

You're going to hate me for saying this, but we need autopilot tests for this. This should be very easy to test with autopilot. I believe the only thing missing is a way to tell if the shortcut overlay is showing (which would be helpful for other AP tests as well).

review: Needs Fixing
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Posted in a previous version of this proposal

You're right, but Thomi, can't we just merge this for now and then performing this manual tests after the freeze?

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote : Posted in a previous version of this proposal

I think the right question there is "why?". Is there any emergency to have that landed to trunk without any test and decide that we will make the tests later on and merged them?

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Posted in a previous version of this proposal

Well, a part on the fact that this was already on trunk (and covered by manual tests) and that I just had to re-fix it after the recent merges... Anyway since this is not a feature, you're right we don't have urgency... But for some features, pending maybe we should try to land them before the freeze (with manual tests, of course) and focusing on writing tests after.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Posted in a previous version of this proposal

Ok, I've finally made the autopilot tests for this, and also I've covered some other manual tests I wrote for the SuperTab switcher.

FYI, this is redundant, but I wanted it like that, to be sure we're getting the same launcher that the controller uses.
71 + .add("keyboard_launcher", pimpl->launchers[pimpl->MonitorWithMouse()]->monitor);

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote : Posted in a previous version of this proposal

Looks good! Please make sure all these new tests pass for you.

review: Approve
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Looks good to me.

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

No proposals found for merge of lp:~thomir/unity/fix-ap-test-stability into lp:unity.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.