unity-2d

Merge lp://staging/~sladen/unity-2d/unity-2d-no-glow-lp933578 into lp://staging/unity-2d

unity-2d-no-glow-lp933578
Merge into trunk

Proposed by Paul Sladen on 2012-02-24

Status:

Merged

Approved by:

Gerry Boland on 2012-02-27

Approved revision:

945

Merged at revision:

937

Proposed branch:

lp://staging/~sladen/unity-2d/unity-2d-no-glow-lp933578

Merge into:

lp://staging/unity-2d

Diff against target:

170 lines (+2/-64)

8 files modified

shell/common/SearchEntry.qml (+0/-10)
shell/common/artwork/desktop_dash_background.sci (+2/-2)
shell/dash/CategoryHeader.qml (+0/-16)
shell/dash/LensBar.qml (+0/-6)
shell/dash/LensButton.qml (+0/-8)
shell/dash/MultiRangeButton.qml (+0/-7)
shell/dash/Star.qml (+0/-7)
shell/dash/TickBox.qml (+0/-8)

To merge this branch:

bzr merge lp://staging/~sladen/unity-2d/unity-2d-no-glow-lp933578

Undecided

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Gerry Boland (community)	functional	2012-02-24	Approve on 2012-02-27
Xi Zhu (community)	design		Approve on 2012-02-27
Michał Sawicz		2012-02-24	Needs Fixing on 2012-02-24
Paul Sladen			Pending
Review via email: mp+94597@code.staging.launchpad.net

This proposal supersedes a proposal from 2012-02-24.

Description of the change

No Glow assets + fixups for (LP: #933578)
This includes replacements assets for:
  Pips + arrows
  Dash search box and search box status icons
  Dash edge background (transparent)
  Dash edge background (non-transparent)
With associated code changes:
  Removal of blurRadius except for background itself
  Background tiling phase realignment for transparent background

Revision history for this message

Michał Sawicz (saviq) wrote on 2012-02-24: Posted in a previous version of this proposal

Just removing "blurRadius" from DropShadows means the shadow will actually be there, and drawn, just that it won't blur. So you end up with a (expensive) copy of the text behind it.

Please completely remove the whole "effect: DropShadow {...}" properties to actually get rid of the shadows.

review: Needs Fixing

Revision history for this message

Michał Sawicz (saviq) wrote on 2012-02-24: Posted in a previous version of this proposal

Please also provide a before and after images for design sign-off.

review: Needs Fixing

Revision history for this message

Michał Sawicz (saviq) wrote on 2012-02-24: Posted in a previous version of this proposal

We need a way to indicate focus on the "Filter results", it was only being highlighted with the glow, now it's too easy to get "lost" in the dash when keyboard-navigating.

We have a similar issue with category headers that don't have "expand" links, so consistency here would probably make sense (not "it's broken" consistency, but a way to indicate focus on them).

review: Needs Fixing

Revision history for this message

Paul Sladen (sladen) wrote on 2012-02-24: Posted in a previous version of this proposal

I believe in all of these locations hover/focus is indicated by colour:white && blurRadius. The colour: white is still there and continues to provide the primary highlight.

There is a design solution for replacing the white highlight with a full-width background grey highlight, and this is in Unity 3D. However, Gerry requested that the merge say on focus and be a minimal diff without other changes other than removing the glow from the assets and code.

Revision history for this message

Paul Sladen (sladen) wrote on 2012-02-24: Posted in a previous version of this proposal

Before, After, and Delta screenshots attached to parent bug report.

review: Needs Resubmitting

Revision history for this message

Michał Sawicz (saviq) wrote on 2012-02-24: Posted in a previous version of this proposal

> I believe in all of these locations hover/focus is indicated by colour:white
> && blurRadius. The colour: white is still there and continues to provide the
> primary highlight.

> There is a design solution for replacing the white highlight with a full-width
> background grey highlight, and this is in Unity 3D.

Except that the label I mentioned ("Filter results") is already white, so we lose
focus indication completely. Please leave that one instance of the glow in place,
we'll fix the whole approach to focus in dash in a separate MR.

> However, Gerry requested that the merge say on focus and be a minimal diff
> without other changes other than removing the glow from the assets and code.

That's ok, but just removing the blurRadius property doesn't make sense. The whole
"effect: DropShadow { ... }" component is responsible for just the glow, so to remove
the glow we need to remove the whole of it.

Revision history for this message

Paul Sladen (sladen) wrote on 2012-02-24: Posted in a previous version of this proposal

Done on both accounts.

review: Needs Resubmitting

Revision history for this message

Michał Sawicz (saviq) wrote on 2012-02-24:

One last thing:
=== renamed file 'shell/launcher/artwork/launcher_pip_ltr.png' => 'shell/launcher/artwork/launcher_pip_ltr.png.OTHER'

That doesn't look right?

review: Needs Fixing

lp://staging/~sladen/unity-2d/unity-2d-no-glow-lp933578 updated on 2012-02-27

944. By Paul Sladen on 2012-02-27: Merge to HEAD; manually resolve binary merge conflict:
shell/launcher/artwork/launcher_pip_ltr.png
945. By Paul Sladen on 2012-02-27: Restore property moving && associated workaround warnings