810340 - Downloads button changes size on OSX depending on the time remaining in a download

Status: RESOLVED FIXED

(Firefox :: Downloads Panel, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Bug 794752 fixed the problem where the downloads button resized itself on first click.

Now it looks like the OSX button isn't wide enough for its potential contents. I'll attach a screenshot with an example.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Demonstration of the problem

Comment 2

[Mike Conley (:mconley) (:⚙️)]

This could be solved by bug 801832.

Comment 3

[Marco Bonardo [:mak]]

yes, otherwise we should set again a min-width, but this time properly, on both the placeholder and the indicator.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

(In reply to Marco Bonardo [:mak] from comment #3)
> yes, otherwise we should set again a min-width, but this time properly, on
> both the placeholder and the indicator.

Let's just do this for now.

It might be strange for the button to be slightly wider than the "Home" button, but not as wide as the "Bookmarks" button. Should we try to make it the same width as the Bookmarks button?

Comment 5

[Marco Bonardo [:mak]]

There is a third alternative, reduce contents size just a bit, maybe it's enough to slightly reduce time left font-size.

I'm not sure how much wider the button should be honestly, it's a new kind of magic button...

Comment 6

[Marco Bonardo [:mak]]

or we may ask mstange or dao if there's possibility to enlarge all the buttons by a couple pixels, I don't know what the current width is based on

Comment 7

[Mike Conley (:mconley) (:⚙️)]

(In reply to Marco Bonardo [:mak] from comment #5)
> There is a third alternative, reduce contents size just a bit, maybe it's
> enough to slightly reduce time left font-size.

I just tried reducing the font-size, and I'm actually quite pleased with the result. I'll post up a new patch and some screenshots.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

Screenshot coming.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Attachment: Screenshot with patch applied

"m" was the widest character of the set that goes next two the two digits, and that one doesn't seem to cause the button expansion anymore.

Comment 10

[Marco Bonardo [:mak]]

Comment on attachment 680289 [details] [diff] [review]
Patch v1

Review of attachment 680289 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/pinstripe/downloads/downloads.css
@@ +240,5 @@
>    margin: -3px 0 0;
>    color: hsl(0,0%,30%);
>    text-shadow: 0 1px 0 hsla(0,0%,100%,.5);
> +  font-size: 9px;
> +  line-height: 9px;

do you feel like the button contents look centered by using it?
This seems to push the progressbar up by 1 px by reducing the line-height, and looking at the sshot with rulers, the bottom light shadow of the progress bar is indeed 1px higher than the bottom light shadow of the arrow. I'd not want to have content packed up at the top of the button, that may look weird.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Attachment: Widening the progress bar

Hm - alright, I see your point there.

We could take this opportunity to increase the height and visibility of the progress bar (which, IMO, is pretty thin on OSX).

See screenshot.

Thoughts?

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Attachment: More context

Here's a screenshot with a bit more context, zoomed out.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v2

This patch was used for my last two screenshots (attachment 680378 [details] and attachment 680379 [details]).

Comment 14

[Marco Bonardo [:mak]]

Comment on attachment 680380 [details] [diff] [review]
Patch v2

Review of attachment 680380 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/pinstripe/downloads/downloads.css
@@ +246,5 @@
>  }
>  
>  #downloads-indicator-progress {
>    width: 16px;
> +  height: 5px;

This looks fine, but please check we are not breaking the Pause state now, when we show the arrow instead of the counter, since the progress is taller it may touch the arrow and you may have to reduce it by 1px as well...

Comment 15

[Mike Conley (:mconley) (:⚙️)]

(In reply to Marco Bonardo [:mak] from comment #14)
> Comment on attachment 680380 [details] [diff] [review]
> Patch v2
> 
> Review of attachment 680380 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/pinstripe/downloads/downloads.css
> @@ +246,5 @@
> >  }
> >  
> >  #downloads-indicator-progress {
> >    width: 16px;
> > +  height: 5px;
> 
> This looks fine, but please check we are not breaking the Pause state now,
> when we show the arrow instead of the counter, since the progress is taller
> it may touch the arrow and you may have to reduce it by 1px as well...

Paused looks fine - lots of clearance from the tip of the arrow to the progressmeter. Thanks for the r+!

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Landed on mozilla-inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/30df269639f0

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/30df269639f0