Merge lp:~unity-team/unity/autohide-load-934514 into lp:unity

Proposed by Bilal Akhtar
Status: Needs review
Proposed branch: lp:~unity-team/unity/autohide-load-934514
Merge into: lp:unity
Diff against target: 56 lines (+29/-0)
2 files modified
launcher/LauncherController.cpp (+10/-0)
manual-tests/Launcher.txt (+19/-0)
To merge this branch: bzr merge lp:~unity-team/unity/autohide-load-934514
Reviewer Review Type Date Requested Status
Thomi Richards (community) quality Approve
Tim Penhey (community) Needs Information
Review via email: mp+117367@code.launchpad.net

Commit message

LauncherController: Don't hide the launcher for 3 seconds after it finishes loading on login (LP: #934513)

Description of the change

Fixes bug #934513 by ensuring the launcher doesn't start hidden, but instead hides 3 seconds after starting up (when autohide is enabled, that is).

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Do we care about the situation where someone has autohide enabled, and moves the mouse over the launcher within the first three seconds of being logged in?

review: Needs Information
Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

Yes, this works. I just re-built and re-tested it, and if the user moves the mouse over the launcher, the ForceReveal(false) does nothing until the mouse moves away from the launcher.

Revision history for this message
Tim Penhey (thumper) wrote :

OK. Since this is a "start unity" type test, can I get you to write a manual test for it?

review: Needs Fixing
Revision history for this message
Tim Penhey (thumper) :
review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

No commit message specified.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

 auto hide_all_launchers = [&]()
10 + {
11 + for (auto launcher : launchers)
12 + launcher->ForceReveal(false);
13 + return true;
14 + };
15 + sources_.AddTimeout(3000, hide_all_launchers);

Whi is this timeout always running every 3 seconds?
I'm not sure this is really needed (or a good thing).

Also, probably you could add a sort of unit tests in test_launcher_controller (checking if the force-reveal quirk is correctly set on the launcher's hide machine)...

Revision history for this message
Tim Penhey (thumper) wrote :

Marco, could you provide more of a guideline for an automated test?

review: Needs Information
Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

Marco, the test doesn't run every three seconds. Once the "return true" call is reached, the timeout stops running. So it runs only once.

Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

Actually, I was wrong. I need to return false rather than true. Change made.

2533. By Bilal Akhtar

Change "return true" to "return false" to destroy timeout

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

Has conflicts with trunk. needs re-merging.

2534. By Bilal Akhtar

Merge from trunk and fix conflicts

Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

Re-merged, fixed conflicts.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Are 3 seconds enough when loading unity on startup? Couldn't this timeout happen when the interface is still loading?

PS: You can use AddTimeoutSeconds and maybe reduce the timeout priority.

Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

Not sure; John preferred to go for 3 seconds (see the description of the linked bug report).

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

From a quality POV, I'm happy that this code is merged with only a manual test.

review: Approve (quality)

Unmerged revisions

2534. By Bilal Akhtar

Merge from trunk and fix conflicts

2533. By Bilal Akhtar

Change "return true" to "return false" to destroy timeout

2532. By Bilal Akhtar

Add manual test for login autohide change

2531. By Bilal Akhtar

Merge from trunk

2530. By Bilal Akhtar

Don't hide the launcher for 3 seconds after it finishes loading on login (LP: #934513)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'launcher/LauncherController.cpp'
--- launcher/LauncherController.cpp 2012-08-30 21:09:02 +0000
+++ launcher/LauncherController.cpp 2012-09-04 08:17:19 +0000
@@ -173,6 +173,15 @@
173 launcher->QueueDraw();173 launcher->QueueDraw();
174 });174 });
175175
176 // Hide launcher after 3 seconds
177 auto hide_all_launchers = [&]()
178 {
179 for (auto launcher : launchers)
180 launcher->ForceReveal(false);
181 return false;
182 };
183 sources_.AddTimeout(3000, hide_all_launchers);
184
176 dbus_owner_ = g_bus_own_name(G_BUS_TYPE_SESSION, DBUS_NAME.c_str(), G_BUS_NAME_OWNER_FLAGS_NONE,185 dbus_owner_ = g_bus_own_name(G_BUS_TYPE_SESSION, DBUS_NAME.c_str(), G_BUS_NAME_OWNER_FLAGS_NONE,
177 OnBusAcquired, nullptr, nullptr, this, nullptr);186 OnBusAcquired, nullptr, nullptr, this, nullptr);
178}187}
@@ -219,6 +228,7 @@
219 launchers[i]->monitor(monitor);228 launchers[i]->monitor(monitor);
220 launchers[i]->Resize();229 launchers[i]->Resize();
221 edge_barriers_.Subscribe(launchers[i].GetPointer(), launchers[i]->monitor);230 edge_barriers_.Subscribe(launchers[i].GetPointer(), launchers[i]->monitor);
231 launchers[i]->ForceReveal(true);
222 }232 }
223233
224 for (unsigned int i = last_launcher; i < launchers_size; ++i)234 for (unsigned int i = last_launcher; i < launchers_size; ++i)
225235
=== modified file 'manual-tests/Launcher.txt'
--- manual-tests/Launcher.txt 2012-09-03 00:07:32 +0000
+++ manual-tests/Launcher.txt 2012-09-04 08:17:19 +0000
@@ -703,6 +703,25 @@
703 * See https://bugs.launchpad.net/unity/+bug/980942/+attachment/3059914/+files/launcher-accordion-effect-tooltip-bug.ogv.703 * See https://bugs.launchpad.net/unity/+bug/980942/+attachment/3059914/+files/launcher-accordion-effect-tooltip-bug.ogv.
704704
705705
706Test launcher autohide on login
707------------------------------------
708
709Setup:
710 * Ensure launcher autohide is enabled.
711
712Action:
713 * Log into Unity
714 * Immediately after the launcher loads, move the mouse over it
715 * Keep mouse over launcher for more than three seconds
716 * Move mouse away from launcher
717
718Expected results:
719 * The launcher loads in a revealed (not hidden) state.
720 * If the user takes no action, the launcher hides after three seconds
721 * Moving the mouse over the launcher prevents the launcher from hiding
722 * After the user moves the mouse away, the launcher hides
723
724
706Stop SD card by dragging it's icon to trash725Stop SD card by dragging it's icon to trash
707---------------------------------------------------726---------------------------------------------------
708727