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
1=== modified file 'launcher/LauncherController.cpp'
2--- launcher/LauncherController.cpp 2012-08-30 21:09:02 +0000
3+++ launcher/LauncherController.cpp 2012-09-04 08:17:19 +0000
4@@ -173,6 +173,15 @@
5 launcher->QueueDraw();
6 });
7
8+ // Hide launcher after 3 seconds
9+ auto hide_all_launchers = [&]()
10+ {
11+ for (auto launcher : launchers)
12+ launcher->ForceReveal(false);
13+ return false;
14+ };
15+ sources_.AddTimeout(3000, hide_all_launchers);
16+
17 dbus_owner_ = g_bus_own_name(G_BUS_TYPE_SESSION, DBUS_NAME.c_str(), G_BUS_NAME_OWNER_FLAGS_NONE,
18 OnBusAcquired, nullptr, nullptr, this, nullptr);
19 }
20@@ -219,6 +228,7 @@
21 launchers[i]->monitor(monitor);
22 launchers[i]->Resize();
23 edge_barriers_.Subscribe(launchers[i].GetPointer(), launchers[i]->monitor);
24+ launchers[i]->ForceReveal(true);
25 }
26
27 for (unsigned int i = last_launcher; i < launchers_size; ++i)
28
29=== modified file 'manual-tests/Launcher.txt'
30--- manual-tests/Launcher.txt 2012-09-03 00:07:32 +0000
31+++ manual-tests/Launcher.txt 2012-09-04 08:17:19 +0000
32@@ -703,6 +703,25 @@
33 * See https://bugs.launchpad.net/unity/+bug/980942/+attachment/3059914/+files/launcher-accordion-effect-tooltip-bug.ogv.
34
35
36+Test launcher autohide on login
37+------------------------------------
38+
39+Setup:
40+ * Ensure launcher autohide is enabled.
41+
42+Action:
43+ * Log into Unity
44+ * Immediately after the launcher loads, move the mouse over it
45+ * Keep mouse over launcher for more than three seconds
46+ * Move mouse away from launcher
47+
48+Expected results:
49+ * The launcher loads in a revealed (not hidden) state.
50+ * If the user takes no action, the launcher hides after three seconds
51+ * Moving the mouse over the launcher prevents the launcher from hiding
52+ * After the user moves the mouse away, the launcher hides
53+
54+
55 Stop SD card by dragging it's icon to trash
56 ---------------------------------------------------
57