Unity

Merge lp://qastaging/~3v1n0/unity/hud-padding-fixes into lp://qastaging/unity

  1. hud-padding-fixes
  2. Merge into trunk
Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 2202
Proposed branch: lp://qastaging/~3v1n0/unity/hud-padding-fixes
Merge into: lp://qastaging/unity
Diff against target: 327 lines (+45/-92)
6 files modified
plugins/unityshell/src/HudController.cpp (+1/-1)
plugins/unityshell/src/HudIcon.cpp (+11/-18)
plugins/unityshell/src/HudIcon.h (+2/-19)
plugins/unityshell/src/HudView.cpp (+24/-47)
plugins/unityshell/src/HudView.h (+0/-6)
plugins/unityshell/src/IconTexture.cpp (+7/-1)
To merge this branch: bzr merge lp://qastaging/~3v1n0/unity/hud-padding-fixes
Reviewer Review Type Date Requested Status
Andrea Cimitan (community) design Approve
Tim Penhey (community) Approve
Thomi Richards (community) Needs Fixing
Review via email: mp+100317@code.qastaging.launchpad.net

Commit message

Hud: fixed the paddings and geometries, remove the icon glitches

I've fixed all the paddings and geometries to match design (and the dash, then), plus I've fixed a size mismatch that caused some drawing glitches of the icon.

HUD before the fix: http://i.imgur.com/b7BO0.png | http://i.imgur.com/9vpmv.png
HUD after the fix: http://i.imgur.com/wDxPO.png | http://i.imgur.com/kSLch.png

Description of the change

The HUD padding was not correct and this caused it not to match the dash geometries, especially when using a locked launcher.

I've fixed all the paddings and geometries to match design (and the dash, then), plus I've fixed a size mismatch that caused some drawing glitches of the icon.

HUD before the fix: http://i.imgur.com/b7BO0.png | http://i.imgur.com/9vpmv.png
HUD after the fix: http://i.imgur.com/wDxPO.png | http://i.imgur.com/kSLch.png

To post a comment you must log in.
Revision history for this message
Andrea Cimitan (cimi) :
review: Approve (design)
Revision history for this message
Andrea Cimitan (cimi) :
review: Needs Resubmitting
Revision history for this message
Andrea Cimitan (cimi) :
review: Abstain
Revision history for this message
Andrea Cimitan (cimi) wrote :

how can I cancel my review? :) The hud visuals seems fine, but you're affecting the launcher, so this seems leading to regressions. But I didn't test on my pc to prove them

Revision history for this message
Andrea Cimitan (cimi) wrote :

Looking closely it seems you're right, there's 1px gap between the hud and the launcher. I should not do reviews on sunday morning - I will compile and test tomorrow

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

> how can I cancel my review? :) The hud visuals seems fine, but you're
> affecting the launcher, so this seems leading to regressions.

I'm not modifying the launcher, I've modified the default parameter used by the HUD to set the launcher size.
But that parameter was wrong (maybe it has been put there as workaround to make the HUD to be close to the dash geometries).

> Looking closely it seems you're right, there's 1px gap between the hud and the launcher.

That's what I meant. Compare my two screenshots, this is totally wrong.

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

Needs a test.

Should be pretty easy to construct an autopilot test that changes the launcher to different sizes, reveals the hud, and checks that it's X position is correct?

Cheers,

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

Thomi, this is a pure visual change, so tests are not needed.

Of course it's possible to integrate some checks in AP, but this shouldn't be required.

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

I'm OK with these changes. I was talking with Thomi about the tests. While it is desirable to have some AP tests around the layout, which we should be able to do relatively easily, I don't want to change the goal posts during the last four weeks of the cycle, so this can land without tests due to the visual nature of the change.

In the future (i.e. next cycle), I expect that we'll start to add some AP tests around layout.

review: Approve
Revision history for this message
Andrea Cimitan (cimi) :
review: Approve (design)

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.