Unity

Merge lp://qastaging/~nick-dedekind/unity/lp923657.dash-dismiss into lp://qastaging/unity

lp923657.dash-dismiss
Merge into trunk

Proposed by Nick Dedekind on 2012-09-06

Status:

Merged

Approved by:

Brandon Schaefer on 2012-09-12

Approved revision:

no longer in the source branch.

Merged at revision:

2684

Proposed branch:

lp://qastaging/~nick-dedekind/unity/lp923657.dash-dismiss

Merge into:

lp://qastaging/unity

Diff against target:

367 lines (+144/-50)

8 files modified

dash/DashController.cpp (+8/-10)
dash/DashController.h (+2/-1)
launcher/LauncherController.cpp (+1/-1)
manual-tests/Dash.txt (+13/-0)
plugins/unityshell/src/unityshell.cpp (+13/-4)
plugins/unityshell/src/unityshell.h (+1/-1)
tests/autopilot/unity/tests/test_dash.py (+55/-17)
tests/autopilot/unity/tests/test_hud.py (+51/-16)

To merge this branch:

bzr merge lp://qastaging/~nick-dedekind/unity/lp923657.dash-dismiss

Related bugs:

Bug #921512: HUD, Multi-monitor - HUD doesn't get dismissed if you click in the other monitor	Medium	Fix Released
Bug #923657: Dash, multi-monitor: Dash is not dismissed when something in the other display is clicked	Critical	Fix Released
Bug #1044086: When opening and then closing the dash quickly the panel is too slow to update	Medium	Fix Released
Bug #1049593: [FFE][UIFE]Dash - Finesse the placement, movement and behaviour of the 12.10 Dash	Critical	Fix Released

Link a bug report

Reviewer	Date Requested	Status
Brandon Schaefer (community)		Approve on 2012-09-12
Christopher Lee (community)		Needs Fixing on 2012-09-11
Andrea Azzarone (community)	2012-09-06	Needs Fixing on 2012-09-11
Review via email: mp+123128@code.qastaging.launchpad.net

Commit message

Fixed dash dismissal when changing focus to a window on monitor external to that of the dash. (LP#923657)
Improved panel update speed when opening dash. (LP#1044086)

Description of the change

Fixed dash dismissal when changing focus to a window on monitor external to that of the dash. (LP#923657)
Improved panel update speed when opening dash. (LP#1044086)

Revision history for this message

Brandon Schaefer (brandontschaefer) wrote on 2012-09-06:

Looks good, all that is missing are tests!

Revision history for this message

Andrea Azzarone (azzar1) wrote on 2012-09-06:

# Open the terminal.
# Run: sleep 10; gnome-calc
# Open the dash.

After 10 seconds, gnome-calc is opened and dash is dismissed. By design window opened without user interaction (see update-manager windows) should not steal focus (or dismiss the dash).

review: Needs Fixing

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2012-09-07:

Added AP tests.
Also fixed a drawing issue where if you clicked across monitors for dash/hud closure, the dash/hud would intermittently be drawn on the monitor where the mouse is located.

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2012-09-11:

Right, now the dash will not close on an async app open when the dash is open, but will close when mouse is clicked on another desktop.

Revision history for this message

Andrea Azzarone (azzar1) wrote on 2012-09-11:

86 +
87 +Test the Panel does not lose track of when the Dash is opened then closed.
88 +--------------------------------------------------------------------------
89 +This tests shows the panel will not think the Dash is opened when it is closed.
90 +(see lp:1044086)
91 +
92 +Setup:

Please use Actions:

93 +#. ress Super twice (Quickly)
94 +
95 +Expected Result:
96 + The panel should look the same as if you had never opened the dash.
97 +
98 +Wrong Result:
99 + The panel still thinks the dash is open and is blured to match your background.

Also Wrong Result section is not listed in TEST_TEMPLATE.txt

The cpp looks good to me and works here. Ask thomi to review the AP test.

review: Needs Fixing

Revision history for this message

Christopher Lee (veebers) wrote on 2012-09-11:

Reviewing the AP test, instead of thomi :)
I'm informed that the merge conflict is being fixed up already.

A couple of minor things:
202 + prev_monitor = None
  - Never actually used
203 + .. range(0, self...
  - The 0 isn't required, start defaults to 0
204 +
  - Extra newline
206 + self.dash.ensure_visible()
  - Please also add self.addCleanup(self.dash.ensure_hidden) to make sure things are cleaned up in case of any issues.

Otherwise looks good.

Revision history for this message

Christopher Lee (veebers) wrote on 2012-09-11:

Oops, wrong comment type before :P

review: Needs Fixing

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2012-09-12:

Not sure how the setup/teaddown work for each test, but don't they run after each test? meaning that ensure_hidden is called after each test.
Anyways, I've added the ensure_hidden cleanup to the tests.

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2012-09-12:

I can't seem to test the new Hud tests. Keybindings aren't working correctly, when the keypress happens I get some garbage in the stdout and failed test when the hud fails to open.

Can some-one test the hud AP tests for me?

Revision history for this message

Brandon Schaefer (brandontschaefer) wrote on 2012-09-12:

These tests pass, the code looks good :). +1.

review: Approve