Closed Bug 1402578 Opened 4 years ago Closed 4 years ago

Port bug 1401917 to TB [Let tab strip scroll buttons use --toolbarbutton-icon-fill-opacity]

Categories

(Thunderbird :: Theme, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 58.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(4 files, 2 obsolete files)

With bug 1401917 FX uses the fill-opacity in the arrow-left.svg to set the opacity. With this we don't need to set the opacity of the whole button.
Attached patch scrollbutton.patch (obsolete) — Splinter Review
This is a port of https://hg.mozilla.org/mozilla-central/rev/32d758581c46 but we need also the new SVG file and define the new variable which FX already had for their buttons.

I've set the 'color: inherit;' to the scrollbuttons because until now the disabled scrollbuttons used graytext instead the toolbar color. This together with the opacity made the disabled button almost invisible.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8911454 - Flags: review?(jorgk)
Looks like another "spot the difference" review. Is there a visible difference?

Since my time is limited, maybe you can tell me where to look. Looking at the CSS in the DOM Inspector (computed style, list style image), I can see that for both tab bar arrows arrow-left.svg is used, so perhaps one is mirrored, yes?
You need to open many tabs until the overflow arrows appears. This arrows are the subject of this patch. The only difference is the disabled arrow button there without patch the arrow is grey instead a more transparent toolbar color (when window active white or black, depending the accent color, and on inactive window black).
Comment on attachment 8911454 [details] [diff] [review]
scrollbutton.patch

I can see two effects with this patch:
The inactive arrow is better visible. With an accent colour and tabs not in title bar, it's almost invisible without the patch.

The active arrow is less pronounced now. I'm not sure I like that. I think it needs to look the same as the down-arrow. I'll attach a screenshot.
Attached image screenshot
Left to right:
Inactive - active - down arrow.
Inactive and active are barely different, the down arrow is more pronounced.
Attached patch scrollbutton.patch (obsolete) — Splinter Review
Made active arrow more opaque. All platforms use now the Linux 0.85 instead of 0.7 on OSX and Windows. This makes the difference between enabled and disabled better visible. Also the dropdown arrow has now the fill-opacity to look like the other arrows.
Attachment #8911454 - Attachment is obsolete: true
Attachment #8911454 - Flags: review?(jorgk)
Attachment #8911539 - Flags: review?(jorgk)
Attached image Another screenshot
Hmm, what I see now compared to the previous patch:
Inactive arrow: the same.
Active arrow: a bit brighter.
Down-arrow: less bright.

Why can't we leave the down-arrow as it was and make the active arrow the same? Next to the bright calendar icons, I don't understand why the arrows are so pale. They are also paler than the tab icons and the menu text.
(In reply to Jorg K (GMT+2) from comment #7)
> Why can't we leave the down-arrow as it was and make the active arrow the
> same? Next to the bright calendar icons, I don't understand why the arrows
> are so pale. They are also paler than the tab icons and the menu text.

The Calendar icons are brighter because they don't use the opacity yet.

I've set the opacity now to 1. When I implement the icons like FX, I'll set the opacity back to 0.85. Then the icons and arrows have the same brightness. Sounds okay?
Attachment #8911539 - Attachment is obsolete: true
Attachment #8911539 - Flags: review?(jorgk)
Attachment #8911559 - Flags: review?(jorgk)
Comment on attachment 8911559 [details] [diff] [review]
scrollbutton.patch

(In reply to Richard Marti (:Paenglab) from comment #8)
> I've set the opacity now to 1.
Good, thanks.

> When I implement the icons like FX, I'll set
> the opacity back to 0.85. Then the icons and arrows have the same
> brightness. Sounds okay?
I'd have to see that. If you magnify the arrows, you'll see that very few pixels have the original colour, the rest is already blended into the background. Even now with opacity 1, the arrows are dimmer than the icons which have solid areas of white.
Attachment #8911559 - Flags: review?(jorgk) → review+
Attached image Magnified screenshot
This shows what I mean: Arrow still dim compared to icon.
Attachment #8911513 - Attachment description: screen shot → screenshot
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/eb81a4091d3c
Port bug 1401917 to TB [Let tab strip scroll buttons use --toolbarbutton-icon-fill-opacity]. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 58.0
You need to log in before you can comment on or make changes to this bug.