1139949 - Make the small swatch of current theme in the themes button smaller and rounded.

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Blake Winton (:bwinton) (:☕️)]

Having the swatch is a nice improvement, but it's a little too big.
https://dl.dropboxusercontent.com/u/2301433/ThemeSwatch.png

And while we're modifying the swatch style, it would be nice to have a 3 pixel border-radius, to match the button.  :)

Farhaan, you've worked in this code before; did you want to take a shot at quickly fixing this one?

Comment 1

[Blake Winton (:bwinton) (:☕️)]

(Although, it looks like the border-radius doesn't do anything to the image, so that part might not be possible.)

Comment 2

[farhaan]

yeah sure !! with your guidance ;)

Comment 3

[farhaan]

assign it to me Blake , I would like to work on it for sure.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

Awesome!  Do you remember where the css is from the previous swatch bug?

Comment 5

[farhaan]

yup i remember i guess

Comment 6

[Tim Nguyen :ntim]

(In reply to Blake Winton (:bwinton) from comment #1)
> (Although, it looks like the border-radius doesn't do anything to the image,
> so that part might not be possible.)

overflow: hidden might work.

Comment 7

[farhaan]

Ok i will try and let you know, sorry for not updating this long !!

Comment 8

[:Gijs (he/him)]

[Tracking Requested - why for this release]:

This is pretty visible on customize mode (esp. on retina where the image is also pixelly) and we should fix it before releasing 39.

Comment 9

[farhaan]

None of the css property is affecting the swatch, I will be glad if someone can lend me a bit help on this one.
I tried a lot of things link of which i will put up. Help required

Comment 10

[farhaan]

I tried a lot of stuff but nothing is affecting that swatch need help to resolve this.
List of things I tried:

https://pastebin.mozilla.org/8826870

Comment 11

[Blake Winton (:bwinton) (:☕️)]

Hey Farhaan,

Jared and I have both tried to get the rounded corners working, and they don't seem to be applying for either of us.  As a compromise for this version, can we just make the swatch smaller?

In the previous bug, you changed line 149 of browser/themes/shared/customizableui/customizeMode.inc.css to have:
  #customization-lwtheme-button  {
    padding: 2px 7px;
  }

  #customization-lwtheme-button > .box-inherit > .box-inherit > .button-icon {
    -moz-margin-end: 3px;
    height: 22px;
  }
I think this will probably do what we're asking for…

Thanks,
Blake.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

We can probably get the border-radius if we use the clip-path CSS property and an SVG shape. Blake, would you like to look in to this?

Comment 13

[Blake Winton (:bwinton) (:☕️)]

I've got a bunch of reading list transition bugs to do first, but if no-one else picks it up before me, I'll take a stab at it.  (Also, is clip-path and SVG _really_ the best solution here?  Cause that's pretty gross, if it is.  ;)

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

MattN and Gijs were talking about an alternative approach here, http://logs.glob.uno/?c=mozilla%23fx-team&s=24+Mar+2015&e=24+Mar+2015&h=clip-path#c199079

Comment 15

[farhaan]

I guess this works fine please check and let me know if its good!!

Comment 16

[farhaan]

Attachment: Screen Shot 2015-03-26 at 8.57.24 pm.png

Comment 17

[farhaan]

if that is what you need then its done in this way!
https://pastebin.mozilla.org/8827223

Comment 18

[Blake Winton (:bwinton) (:☕️)]

I think that's better than what we currently have…  Philipp, do you think this new smaller size is good enough to ship?

Comment 19

[Blake Winton (:bwinton) (:☕️)]

Attachment: Let's just make the corners rounded…

Hey Farhaan, sorry for stealing the bug like this, but I had a bit of time this afternoon, so I did some hacking and got it looking like this: https://dl.dropboxusercontent.com/u/2301433/Firefox/ThemeButton.png

(I'm also redirecting the review request to Gijs, since this was his suggestion, and since Jaws isn't accepting review requests for a little while.  ;)

Comment 20

[farhaan]

Hey no worries but how did you do it?

Comment 21

[Blake Winton (:bwinton) (:☕️)]

Check the diff!  ;)

(Seriously, take a look at the diff, and see which parts make sense, and which don't, and ask me about the ones you don't understand…  If your question was more "Where did I get the idea?", then the answer is "From the logs Jaws posted in comment 14." :)

Comment 22

[farhaan]

Why the change in javascript part is needed? Now direct me to other bug :P

Comment 23

[Blake Winton (:bwinton) (:☕️)]

The change in javascript is needed to set the background-image style on the sub-element, instead of setting the image attribute on the button.  (If we set the image attribute on the button, we would get two images, one of which wouldn't be rounded. :)

For the next bug, how do you feel about taking another stab at bug 1102710?  Or you could finish off the work NTim started in bug 1111094…

Comment 24

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Comment on attachment 8584680 [details] [diff] [review]
Let's just make the corners rounded…

I didn't apply the patch, but I assume that it looks like the buttons in bwintons screenshots (https://dl.dropboxusercontent.com/u/2301433/Firefox/ThemeButton.png).

That's a definite improvement!

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/012456b15383

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/012456b15383

Comment 27

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8584680 [details] [diff] [review]
Let's just make the corners rounded…

Approval Request Comment
[Feature/regressing bug #]: issue with new feature introduced in bug 1073234
[User impact if declined]: customization mode looks a little uglier on the themes button
[Describe test coverage new/current, TreeHerder]: manual testing, no automated coverage, just a styling change
[Risks and why]: none expected
[String/UUID change made/needed]: none

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8584680 [details] [diff] [review]
Let's just make the corners rounded…

Approving this for aurora. Round away.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bb9ade69b666

Comment 30

[Rezaul Huque Nayeem]

I have successfully reproduce this bug on Firefox nightly 39.0a1 (2015-03-05)

The bug's fix is verified on latest release 40.0 ( build id : 20150807085045)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0