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

Proposed by Paul Sladen
Status: Merged
Approved by: Gerry Boland
Approved revision: 945
Merged at revision: 937
Proposed branch: lp:~sladen/unity-2d/unity-2d-no-glow-lp933578
Merge into: lp: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:~sladen/unity-2d/unity-2d-no-glow-lp933578
Reviewer Review Type Date Requested Status
Gerry Boland (community) functional Approve
Xi Zhu (community) design Approve
Michał Sawicz Needs Fixing
Paul Sladen Pending
Review via email: mp+94597@code.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

To post a comment you must log in.
Revision history for this message
Michał Sawicz (saviq) wrote : 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 : 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 : 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 : 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 : 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 : 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 : Posted in a previous version of this proposal

Done on both accounts.

review: Needs Resubmitting
Revision history for this message
Michał Sawicz (saviq) wrote :

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
944. By Paul Sladen

Merge to HEAD; manually resolve binary merge conflict:
  shell/launcher/artwork/launcher_pip_ltr.png

945. By Paul Sladen

Restore property moving && associated workaround warnings

Revision history for this message
Xi Zhu (xi.zhu) wrote :

No glow is fine.

review: Approve (design)
Revision history for this message
Gerry Boland (gerboland) wrote :

Functional test looks good, code looks ok, approving!

Thank you Paul.
-G

review: Approve (functional)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'shell/common/SearchEntry.qml'
2--- shell/common/SearchEntry.qml 2012-02-15 13:26:26 +0000
3+++ shell/common/SearchEntry.qml 2012-02-27 16:59:19 +0000
4@@ -87,16 +87,6 @@
5 Accessible.name: searchInstructions.text
6 Accessible.role: Accessible.EditableText
7
8- effect: DropShadow {
9- id: glow
10-
11- blurRadius: 4
12- offset.x: 0
13- offset.y: 0
14- color: "white"
15- enabled: searchInput.text != "" || searchInput.inputMethodComposing
16- }
17-
18 anchors.left: searchIcon.right
19 anchors.leftMargin: -5
20 anchors.right: parent.right
21
22=== modified file 'shell/common/artwork/desktop_dash_background.png'
23Binary files shell/common/artwork/desktop_dash_background.png 2012-01-30 17:23:42 +0000 and shell/common/artwork/desktop_dash_background.png 2012-02-27 16:59:19 +0000 differ
24=== modified file 'shell/common/artwork/desktop_dash_background.sci'
25--- shell/common/artwork/desktop_dash_background.sci 2012-01-30 17:23:42 +0000
26+++ shell/common/artwork/desktop_dash_background.sci 2012-02-27 16:59:19 +0000
27@@ -1,7 +1,7 @@
28 border.left: 0
29 border.top: 0
30-border.bottom: 45
31-border.right: 46
32+border.bottom: 43
33+border.right: 44
34 source: desktop_dash_background.png
35 horizontalTileRule: Repeat
36 verticalTileRule: Repeat
37
38=== modified file 'shell/common/artwork/desktop_dash_background_no_transparency.png'
39Binary files shell/common/artwork/desktop_dash_background_no_transparency.png 2012-01-30 17:23:42 +0000 and shell/common/artwork/desktop_dash_background_no_transparency.png 2012-02-27 16:59:19 +0000 differ
40=== modified file 'shell/common/artwork/search_background.png'
41Binary files shell/common/artwork/search_background.png 2012-02-01 23:52:38 +0000 and shell/common/artwork/search_background.png 2012-02-27 16:59:19 +0000 differ
42=== modified file 'shell/common/artwork/search_icon.png'
43Binary files shell/common/artwork/search_icon.png 2012-02-01 23:52:38 +0000 and shell/common/artwork/search_icon.png 2012-02-27 16:59:19 +0000 differ
44=== modified file 'shell/dash/CategoryHeader.qml'
45--- shell/dash/CategoryHeader.qml 2012-01-10 16:52:24 +0000
46+++ shell/dash/CategoryHeader.qml 2012-02-27 16:59:19 +0000
47@@ -36,14 +36,6 @@
48
49 Accessible.name: "%1 %2 %3".arg(title.text).arg(label.text).arg(folded ? u2d.tr("not expanded") : u2d.tr("expanded"))
50
51- effect: DropShadow {
52- blurRadius: 6
53- offset.x: 0
54- offset.y: 0
55- color: "white"
56- enabled: ( categoryHeader.state == "pressed" && !moving )
57- }
58-
59 Image {
60 id: iconImage
61
62@@ -81,14 +73,6 @@
63 || categoryHeader.state == "hovered" ) ? 1.0 : 0.5
64 Behavior on opacity {NumberAnimation { duration: 100 }}
65
66- effect: DropShadow {
67- blurRadius: 4
68- offset.x: 0
69- offset.y: 0
70- color: "white"
71- enabled: ( moreResults.opacity == 1.0 && !moving )
72- }
73-
74 TextCustom {
75 id: label
76
77
78=== modified file 'shell/dash/LensBar.qml'
79--- shell/dash/LensBar.qml 2012-02-10 20:25:00 +0000
80+++ shell/dash/LensBar.qml 2012-02-27 16:59:19 +0000
81@@ -43,12 +43,6 @@
82 }
83
84 /* LensBar contains a row of LensButtons */
85-
86- /* Ugly Hack: the DropShadow effect on the Home lensButton causes visual artifacts as
87- the list of lenses is being populated, when the inter-lensButton spacing is non-zero.
88- A previously painted DropShadow remains in the (transparent) inter-lensButton space.
89- Can work around this by removing spacing and instead add padding to each lensButton.
90- DropShadow not officially supported until Qt4.8, when hopefully this will be fixed. */
91 Row {
92 id: lensContainer
93
94
95=== modified file 'shell/dash/LensButton.qml'
96--- shell/dash/LensButton.qml 2011-12-08 19:16:20 +0000
97+++ shell/dash/LensButton.qml 2012-02-27 16:59:19 +0000
98@@ -31,14 +31,6 @@
99
100 id: lensButton
101
102- effect: DropShadow {
103- blurRadius: 8
104- color: "white"
105- offset.x: 0
106- offset.y: 0
107- enabled: ( lensButton.state == "selected" || active )
108- }
109-
110 Rectangle {
111 anchors.fill: parent
112 anchors.topMargin: 7
113
114=== modified file 'shell/dash/MultiRangeButton.qml'
115--- shell/dash/MultiRangeButton.qml 2011-11-29 17:45:27 +0000
116+++ shell/dash/MultiRangeButton.qml 2012-02-27 16:59:19 +0000
117@@ -43,13 +43,6 @@
118 text: multiRangeButton.text
119 elide: Text.ElideRight
120
121- effect: DropShadow {
122- blurRadius: 8
123- color: "white"
124- offset.x: 0
125- offset.y: 0
126- enabled: ( multiRangeButton.state == "selected" )
127- }
128 }
129
130 Rectangle {
131
132=== modified file 'shell/dash/Star.qml'
133--- shell/dash/Star.qml 2011-11-25 15:44:18 +0000
134+++ shell/dash/Star.qml 2012-02-27 16:59:19 +0000
135@@ -29,13 +29,6 @@
136 width: childrenRect.width
137 height: childrenRect.height
138
139- effect: DropShadow {
140- blurRadius: 8
141- color: "white"
142- offset.x: 0
143- offset.y: 0
144- }
145-
146 Image {
147 width: sourceSize.width
148 height: sourceSize.height
149
150=== modified file 'shell/dash/TickBox.qml'
151--- shell/dash/TickBox.qml 2012-01-10 16:52:24 +0000
152+++ shell/dash/TickBox.qml 2012-02-27 16:59:19 +0000
153@@ -33,14 +33,6 @@
154 width: childrenRect.width
155 height: childrenRect.height
156
157- effect: DropShadow {
158- blurRadius: 8
159- color: "white"
160- offset.x: 0
161- offset.y: 0
162- enabled: ( tickBox.state == "selected" )
163- }
164-
165 Rectangle {
166 id: container
167
168
169=== modified file 'shell/launcher/artwork/launcher_pip_ltr.png'
170Binary files shell/launcher/artwork/launcher_pip_ltr.png 2012-02-18 15:15:31 +0000 and shell/launcher/artwork/launcher_pip_ltr.png 2012-02-27 16:59:19 +0000 differ

Subscribers

People subscribed via source and target branches