Merge lp:~dyams/unity-2d/remove-glow into lp:unity-2d

Proposed by Lohith D Shivamurthy on 2012-02-20
Status: Rejected
Rejected by: Gerry Boland on 2012-02-28
Proposed branch: lp:~dyams/unity-2d/remove-glow
Merge into: lp:unity-2d
Diff against target: 31 lines (+0/-8)
1 file modified
shell/dash/LensButton.qml (+0/-8)
To merge this branch: bzr merge lp:~dyams/unity-2d/remove-glow
Reviewer Review Type Date Requested Status
Gerry Boland (community) 2012-02-20 Disapprove on 2012-02-28
Andrea Cimitan (community) design 2012-02-21 Disapprove on 2012-02-21
Michał Sawicz Needs Information on 2012-02-21
Review via email: mp+93807@code.launchpad.net

Description of the change

[Shell] remove glow

To post a comment you must log in.
Michał Sawicz (saviq) wrote :

Lohith, could you please provide before and after screenshots, so that design can sign it off?

review: Needs Information
Lohith D Shivamurthy (dyams) wrote :

> Lohith, could you please provide before and after screenshots, so that design
> can sign it off?
Saviq, sure
before -> http://ubuntuone.com/2V7wsUuia06PmGRVe6PO4a
after -> http://ubuntuone.com/7b9h9IS7drxbaLQB6h6UE1

Paulo J. S. Silva (pjssilva) wrote :

Sorry,

But I am not sure why I got the message below. I don't have enough
expertise to review a patch to unity-2d (I don't know the codebase).

best,

Paulo

On Tue, Feb 21, 2012 at 10:43 AM, Gerry Boland
<email address hidden> wrote:
> You have been requested to review the proposed merge of lp:~dyams/unity-2d/remove-glow into lp:unity-2d.
>
> For more details, see:
> https://code.launchpad.net/~dyams/unity-2d/remove-glow/+merge/93807
>
> [Shell] remove glow
>
> --
> https://code.launchpad.net/~dyams/unity-2d/remove-glow/+merge/93807
> Your team Ayatana Discussion is requested to review the proposed merge of lp:~dyams/unity-2d/remove-glow into lp:unity-2d.
>
> === modified file 'shell/common/artwork/search_background.png'
> Binary files shell/common/artwork/search_background.png 2012-02-01 23:52:38 +0000 and shell/common/artwork/search_background.png        2012-02-20 11:47:59 +0000 differ
> === modified file 'shell/common/artwork/search_icon.png'
> Binary files shell/common/artwork/search_icon.png       2012-02-01 23:52:38 +0000 and shell/common/artwork/search_icon.png      2012-02-20 11:47:59 +0000 differ
> === modified file 'shell/dash/LensButton.qml'
> --- shell/dash/LensButton.qml   2011-12-08 19:16:20 +0000
> +++ shell/dash/LensButton.qml   2012-02-20 11:47:59 +0000
> @@ -31,14 +31,6 @@
>
>     id: lensButton
>
> -    effect: DropShadow {
> -         blurRadius: 8
> -         color: "white"
> -         offset.x: 0
> -         offset.y: 0
> -         enabled: ( lensButton.state == "selected" || active )
> -    }
> -
>     Rectangle {
>         anchors.fill: parent
>         anchors.topMargin: 7
>
> === modified file 'shell/launcher/artwork/launcher_arrow_ltr.png'
> Binary files shell/launcher/artwork/launcher_arrow_ltr.png      2011-11-11 10:19:14 +0000 and shell/launcher/artwork/launcher_arrow_ltr.png     2012-02-20 11:47:59 +0000 differ
> === modified file 'shell/launcher/artwork/launcher_arrow_outline_ltr.png'
> Binary files shell/launcher/artwork/launcher_arrow_outline_ltr.png      2012-01-23 13:23:59 +0000 and shell/launcher/artwork/launcher_arrow_outline_ltr.png     2012-02-20 11:47:59 +0000 differ
> === modified file 'shell/launcher/artwork/launcher_arrow_rtl.png'
> Binary files shell/launcher/artwork/launcher_arrow_rtl.png      2011-11-11 10:19:14 +0000 and shell/launcher/artwork/launcher_arrow_rtl.png     2012-02-20 11:47:59 +0000 differ
> === modified file 'shell/launcher/artwork/launcher_pip_ltr.png'
> Binary files shell/launcher/artwork/launcher_pip_ltr.png        2011-11-11 10:19:14 +0000 and shell/launcher/artwork/launcher_pip_ltr.png       2012-02-20 11:47:59 +0000 differ
>

--
Paulo José da Silva e Silva
Professor Associado, Dep. de Ciência da Computação
(Associate Professor, Computer Science Dept.)
Universidade de São Paulo - Brazil

e-mail: <email address hidden>         Web: http://www.ime.usp.br/~pjssilva

Andrea Cimitan (cimi) wrote :

You removed the glow, but increased the size of the search box with the pixels previously allocated to the glow effect.
The dimensions of the search box should be the same as before

review: Disapprove (design)

> You removed the glow, but increased the size of the search box with the pixels
> previously allocated to the glow effect.
> The dimensions of the search box should be the same as before

I don't know why I was asked to review this change as well since I have no experience in development for Ubuntu. But I agree on Andrea Cimitan, the dimensions should be the same as before on the search box, otherwise it gets misaligned with the content in the dash.

Leo Barnes (barnes-leo) wrote :

I was also asked to review it but I am not really involved in the Ubuntu development... Bug? ;)

Gerry Boland (gerboland) wrote :

@ Niklas Rosenqvist (niklas-s-rosenqvist)
@ Leo Barnes (barnes-leo)
Sorry guys, I got Launchpad errors when assigning this to design. You must've been randomly targeted :)
-Gerry

Gerry Boland (gerboland) wrote :

This was fixed in another MR, rejecting.

review: Disapprove

Unmerged revisions

919. By Lohith D Shivamurthy on 2012-02-17

remove glow effect

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'shell/common/artwork/search_background.png'
2Binary files shell/common/artwork/search_background.png 2012-02-01 23:52:38 +0000 and shell/common/artwork/search_background.png 2012-02-20 11:47:59 +0000 differ
3=== modified file 'shell/common/artwork/search_icon.png'
4Binary files shell/common/artwork/search_icon.png 2012-02-01 23:52:38 +0000 and shell/common/artwork/search_icon.png 2012-02-20 11:47:59 +0000 differ
5=== modified file 'shell/dash/LensButton.qml'
6--- shell/dash/LensButton.qml 2011-12-08 19:16:20 +0000
7+++ shell/dash/LensButton.qml 2012-02-20 11:47:59 +0000
8@@ -31,14 +31,6 @@
9
10 id: lensButton
11
12- effect: DropShadow {
13- blurRadius: 8
14- color: "white"
15- offset.x: 0
16- offset.y: 0
17- enabled: ( lensButton.state == "selected" || active )
18- }
19-
20 Rectangle {
21 anchors.fill: parent
22 anchors.topMargin: 7
23
24=== modified file 'shell/launcher/artwork/launcher_arrow_ltr.png'
25Binary files shell/launcher/artwork/launcher_arrow_ltr.png 2011-11-11 10:19:14 +0000 and shell/launcher/artwork/launcher_arrow_ltr.png 2012-02-20 11:47:59 +0000 differ
26=== modified file 'shell/launcher/artwork/launcher_arrow_outline_ltr.png'
27Binary files shell/launcher/artwork/launcher_arrow_outline_ltr.png 2012-01-23 13:23:59 +0000 and shell/launcher/artwork/launcher_arrow_outline_ltr.png 2012-02-20 11:47:59 +0000 differ
28=== modified file 'shell/launcher/artwork/launcher_arrow_rtl.png'
29Binary files shell/launcher/artwork/launcher_arrow_rtl.png 2011-11-11 10:19:14 +0000 and shell/launcher/artwork/launcher_arrow_rtl.png 2012-02-20 11:47:59 +0000 differ
30=== modified file 'shell/launcher/artwork/launcher_pip_ltr.png'
31Binary files shell/launcher/artwork/launcher_pip_ltr.png 2011-11-11 10:19:14 +0000 and shell/launcher/artwork/launcher_pip_ltr.png 2012-02-20 11:47:59 +0000 differ

Subscribers

People subscribed via source and target branches