Closed Bug 1139949 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: Toolbars and Customization, defect)

x86
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 40
Tracking Status
firefox39 + fixed
firefox40 --- verified

People

(Reporter: bwinton, Assigned: farhaan.bukhsh, Mentored)

References

Details

(Whiteboard: [qx:spec] [lang=css] [good first bug])

Attachments

(2 files)

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?
(Although, it looks like the border-radius doesn't do anything to the image, so that part might not be possible.)
yeah sure !! with your guidance ;)
assign it to me Blake , I would like to work on it for sure.
Assignee: nobody → farhaan.bukhsh
Status: NEW → ASSIGNED
Awesome!  Do you remember where the css is from the previous swatch bug?
yup i remember i guess
(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.
Ok i will try and let you know, sorry for not updating this long !!
[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.
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
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
Flags: needinfo?(jaws)
Flags: needinfo?(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.
Flags: needinfo?(bwinton)
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?
Flags: needinfo?(jaws) → needinfo?(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.  ;)
Flags: needinfo?(bwinton)
I guess this works fine please check and let me know if its good!!
if that is what you need then its done in this way!
https://pastebin.mozilla.org/8827223
Flags: needinfo?(jaws)
Flags: needinfo?(bwinton)
I think that's better than what we currently have…  Philipp, do you think this new smaller size is good enough to ship?
Flags: needinfo?(bwinton) → needinfo?(philipp)
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.  ;)
Attachment #8584680 - Flags: ui-review?(philipp)
Attachment #8584680 - Flags: review?(gijskruitbosch+bugs)
Hey no worries but how did you do it?
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." :)
Why the change in javascript part is needed? Now direct me to other bug :P
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
Attachment #8584680 - Flags: review?(gijskruitbosch+bugs) → review+
Flags: needinfo?(jaws)
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!
Flags: needinfo?(philipp)
Attachment #8584680 - Flags: ui-review?(philipp) → ui-review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/012456b15383
Keywords: checkin-needed
Whiteboard: [qx] [lang=css] [good first bug] → [qx] [lang=css] [good first bug][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/012456b15383
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [qx] [lang=css] [good first bug][fixed-in-fx-team] → [qx] [lang=css] [good first bug]
Target Milestone: --- → Firefox 40
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
Attachment #8584680 - Flags: approval-mozilla-aurora?
Comment on attachment 8584680 [details] [diff] [review]
Let's just make the corners rounded…

Approving this for aurora. Round away.
Attachment #8584680 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [qx] [lang=css] [good first bug] → [qx:spec] [lang=css] [good first bug]
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
QA Whiteboard: [bugday-20150812]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.