Unity

Merge lp://qastaging/~thumper/unity/lock-out-hud into lp://qastaging/unity

lock-out-hud
Merge into trunk

Proposed by Tim Penhey on 2012-03-21

Status:

Merged

Merge reported by:

Didier Roche-Tolomelli

Merged at revision:

not available

Proposed branch:

lp://qastaging/~thumper/unity/lock-out-hud

Merge into:

lp://qastaging/unity

Diff against target:

1083 lines (+427/-93)

17 files modified

manual-tests/Hud.txt (+14/-0)
plugins/unityshell/src/BFBLauncherIcon.cpp (+27/-1)
plugins/unityshell/src/BFBLauncherIcon.h (+5/-1)
plugins/unityshell/src/HudController.cpp (+34/-11)
plugins/unityshell/src/HudController.h (+4/-0)
plugins/unityshell/src/HudLauncherIcon.cpp (+125/-0)
plugins/unityshell/src/HudLauncherIcon.h (+60/-0)
plugins/unityshell/src/HudView.cpp (+47/-26)
plugins/unityshell/src/HudView.h (+11/-2)
plugins/unityshell/src/Launcher.cpp (+47/-34)
plugins/unityshell/src/Launcher.h (+2/-0)
plugins/unityshell/src/LauncherController.cpp (+13/-1)
plugins/unityshell/src/LauncherHideMachine.cpp (+7/-7)
plugins/unityshell/src/LauncherOptions.h (+10/-10)
plugins/unityshell/src/UBusMessages.h (+5/-0)
plugins/unityshell/src/unityshell.cpp (+5/-0)
tests/autopilot/autopilot/tests/test_hud.py (+11/-0)

To merge this branch:

bzr merge lp://qastaging/~thumper/unity/lock-out-hud

High

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Gord Allott (community)	2012-03-21	Approve on 2012-03-21
Marco Trevisan (Treviño)	2012-03-21	Approve on 2012-03-21
Mirco Müller	2012-03-21	Pending
Review via email: mp+98573@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2012-03-16.

Commit message

Adapt the HUD visuals to look nice with a locked out launcher.

Description of the change

Changes the behaviour of the HUD/Launcher interaction in HideMode: Never

= The Problem =
See: https://bugs.launchpad.net/ayatana-design/+bug/921506

= The Fix =
Modify the Launcher to accept a new LauncherIcon that will listen to a UBUS message dictating its icon

= Testing =
Manual test included, lock-out behaviour is not included in out AP tests so ensuring a valid AP test seems non trivial at this point, requires an AP test module that can modify compiz options

See http://people.canonical.com/~tim/hud.ogv

Revision history for this message

Tim Penhey (thumper) wrote on 2012-03-19: Posted in a previous version of this proposal

Actually adding an autopilot test for this is trivial.

What we can do is to run the HUD AP tests with the launcher in each state, locked out and auto-hide. We already do something like this for launchers for each monitor. I'll tackle the HUD tests with thomi tomorrow.

Revision history for this message

Didier Roche-Tolomelli (didrocks) wrote on 2012-03-20: Posted in a previous version of this proposal

we want that for this release isn't it? can someone review it and merge it with the manual test if anyone has the AP test for it?

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-03-20: Posted in a previous version of this proposal

This has some conflicts with trunk, also you're using some lambda functions to connect to sigc::signal's, but in that case you need to disconnect from them in destructor or use a sigc::mem_fun instead (this doesn't apply to the ubus callback functions, of course).

Plus, I'm not sure if that's wanted, but if enabling the launcher's audohide feature, when closing the HUD, the launcher reveals and then closes again.

42 + if (!g_strcmp0(overlay_identity, "hud"))

Why not using overlay_identity.Str() == "hud" ?

launcher_width.changed.connect([this] (int new_width) { Relayout(); });

I guess you can just pass [&] and please follow what said above. Even if the hud view shouldn't be destroyed, I guess it's better to stay safe.

review: Needs Fixing

Revision history for this message

Tim Penhey (thumper) wrote on 2012-03-21: Posted in a previous version of this proposal

As well as the launcher popping out when you close the hud, if you do the following:
press Alt
press Super

You get the dash with a hidden launcher.

Also, you get a peaking icon when you start the hud with a hidden launcher.

I'm fixing these now. I have a branch that fixes everything so far except making sure the icon saturation is right at the right time.

Revision history for this message

Tim Penhey (thumper) wrote on 2012-03-21: Posted in a previous version of this proposal

Also worth noting I guess, this branch doesn't fix the bug around the icon size for the hud when you don't have a fixed launcher.

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-03-21:

Interactions look good to me now.

We should add AP tests to check also that the launcher status change only as it should when using the HUD...

review: Approve

Revision history for this message

Gord Allott (gordallott) wrote on 2012-03-21:

+1, but technically I'm reviewing the changes Tim made to my code. I also give my code +1 but i may be bias :)

review: Approve

Revision history for this message

Tim Penhey (thumper) wrote on 2012-03-21:

On 21/03/12 23:07, Gord Allott wrote:
> Review: Approve
>
> +1, but technically I'm reviewing the changes Tim made to my code. I also give my code +1 but i may be bias :)

I approved of Gord's work, and extended. So we should have this covered :)

Revision history for this message

Gord Allott (gordallott) wrote on 2012-03-21:

Mr Cimitan is happy with the design, so approving

review: Approve

Revision history for this message

Andrea Cimitan (cimi) wrote on 2012-03-21:

I am fine with the design as a first iteration, two stuffs I noticed and I ask to fix for 12.04:
1) animation of bfb->icon when launcher is on (maybe designers need to provide a desired solution)
2) there's a duplicated separator line between the hud rectangle and the launcher http://i.imgur.com/xgV0L.png
3) the hud icon is using two different tiles when the launcher is on (bfb-like) and the launcher is hidden (normal tile)