Closed Bug 1415019 Opened 2 years ago Closed 2 years ago

Improve background tab loading indicator colors for better contrast and photon color palette compatibility

Categories

(Firefox :: Theme, enhancement, P1)

58 Branch
enhancement

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: sfoster, Assigned: jaws)

References

(Blocks 1 open bug)

Details

(Whiteboard: [reserve-photon-animation])

Attachments

(1 file)

Follow-up from Bug #1408007, in response to :epang's comment:

Currently the light blue looks closer to lavender to me and doesn't really fit into the Photon colour palette.

Also, I agree with Shellye that the grey should be darker so the 2 states are
easier to be distinguished.

Blue: #84c1ff
Grey: #717171

We need to find colors that have sufficient contrast to work against any of the reasonable set of window title and tab background colors, in the default theme. See bug 1396062 for more discussion. 
 
+++ This bug was initially created as a clone of Bug #1408007 +++
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Jared, do you know what is the best way to get a blue titlebar per bug 139606 to test this isn't going to regress there?
Flags: needinfo?(jaws)
two cents:

Like the current dark black to blue & Shiny white in Firefox57 more better as it has correct contrast,

Dark black to Shiny white/Blue for background tabs would be ideal imo as can see the state changes and looks pretty
than the grey to dull blue/lavander
(In reply to Sam Foster [:sfoster] from comment #2)
> Jared, do you know what is the best way to get a blue titlebar per bug
> 139606 to test this isn't going to regress there?

Here's a screencast of how to change the color on Windows 10 (Flash required to view it): https://www.screencast.com/t/cazbfIV7p60Z
Flags: needinfo?(jaws)
(In reply to shellye from comment #3)
> two cents:
> 
> Like the current dark black to blue & Shiny white in Firefox57 more better
> as it has correct contrast,
> 
> Dark black to Shiny white/Blue for background tabs would be ideal imo as can
> see the state changes and looks pretty
> than the grey to dull blue/lavander

This is a tricky one to get right and we're not going to please everyone one, but I have to defer to Eric (UX) on this, for the default theme.
Comment on attachment 8926064 [details]
Bug 1415019 - Tweak the colors for the tab throbber when brighttext is used.

https://reviewboard.mozilla.org/r/197302/#review202960

I'm not able to check this on windows today. I'm assuming you have

::: browser/themes/shared/tabs.inc.css:207
(Diff revision 1)
>    fill: var(--tab-loading-fill);
>    opacity: 1;
>  }
>  
> -#TabsToolbar[brighttext] .tabbrowser-tab:not([visuallyselected=true]) {
> -  --tab-loading-fill: #bbbbff;
> +#TabsToolbar[brighttext] .tab-throbber[progress]:not([visuallyselected=true])::before {
> +  fill: #84c1ff;

Can you explain this change in the commit message? You're changing the fill property of before element rather than the --tab-loading-fill variable?
Attachment #8926064 - Flags: review?(sfoster) → review+
(In reply to Sam Foster [:sfoster] from comment #6)
> Comment on attachment 8926064 [details]
> Bug 1415019 - Tweak the colors for the tab throbber when brighttext is used.
> 
> https://reviewboard.mozilla.org/r/197302/#review202960
> 
> I'm not able to check this on windows today. I'm assuming you have
> 
> ::: browser/themes/shared/tabs.inc.css:207
> (Diff revision 1)
> >    fill: var(--tab-loading-fill);
> >    opacity: 1;
> >  }
> >  
> > -#TabsToolbar[brighttext] .tabbrowser-tab:not([visuallyselected=true]) {
> > -  --tab-loading-fill: #bbbbff;
> > +#TabsToolbar[brighttext] .tab-throbber[progress]:not([visuallyselected=true])::before {
> > +  fill: #84c1ff;
> 
> Can you explain this change in the commit message? You're changing the fill
> property of before element rather than the --tab-loading-fill variable?

This sets the fill the same way that we do for #TabsToolbar[brighttext] .tab-throbber[busy]:not([visuallyselected=true])::before. We don't want to change the variable because this should only affect tabs that are not visually selected. I added a comment for this.
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5f7b033810f5
Tweak the colors for the tab throbber when brighttext is used. r=sfoster
https://hg.mozilla.org/mozilla-central/rev/5f7b033810f5
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Depends on: 1417715
You need to log in before you can comment on or make changes to this bug.