875326 - Polish Australis tab stroke inner highlight

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Matthew N. [:MattN]]

Attachment: v.0.9 Use new images from shorlander, fix layering of ::after, remove tab-active-middle-lwtheme.png

More detailed notes:
* I removed remove tab-active-middle-lwtheme.png because the updated remove tab-active-middle.png images from Stephen don't have the tab fill. This means we will always use fgTabTexture behind the stroke (tab-active-middle.png) rather than having the linear CSS gradient from start/end beside the image gradient in the middle which may not match perfectly.
* deleted redundant styles from browser/themes/linux/browser.css
* consolidate two ".tab-background-middle[selected=true]:-moz-lwtheme" rules

(In reply to Stephen Horlander from bug 858089 comment #17)
> Created attachment 752017 [details]
> Tab Images Linux/OS X
> 
> > Stephen: 
> > 1) What were the tab-texture-*.png images for in browser-aero.css? Was it
> > just that the colour should change in that case? Any reason why we can't
> > just override the colours in the linear-gradient instead of using an image?
> 
> I think it was the only way I could get it to layer properly. If you can do
> it another way it should be fine.

This patch fixes the layering of ::after, as it wasn't above ::before for some reason, which may help. With this patch there is a slight issue with LWT which seems to require a clip-path tweak. I'll do that in a later patch.
 
> > 3) Could you provide the revised tab-stroke-*.png images for OS X (+@2x) and
> > Linux? They should be like Windows where they only contain the stroke and
> > inner highlight.
> 
> Should be able to use these on OS X and Linux.

I thought your Windows images had the highlight but I don't see them now. Are they in some other bug or did I accidentally overwrite them locally?

Comment 1

[Stephen Horlander [:shorlander]]

(In reply to Matthew N. [:MattN] from comment #0)
> This patch fixes the layering of ::after, as it wasn't above ::before for
> some reason, which may help. With this patch there is a slight issue with
> LWT which seems to require a clip-path tweak. I'll do that in a later patch.

Might need to tweak the clip-path and the stroke to make it line up exactly.


> I thought your Windows images had the highlight but I don't see them now.
> Are they in some other bug or did I accidentally overwrite them locally?

The images from the WIP patch (https://bug858089.bugzilla.mozilla.org/attachment.cgi?id=733347) have all of the correct borders and highlights.

Comment 2

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

So, I'm confused here...on Windows, I'm not seeing the inner highlight on the selected tab's curvy strokes:

http://i.imgur.com/VzZdmro.png

I re-applied the tab-stroke-start and tab-stroke-end images that Stephen linked to, and same result.

Is this the way it's supposed to look?

Comment 3

[Stephen Horlander [:shorlander]]

(In reply to Mike Conley (:mconley) from comment #2)
> So, I'm confused here...on Windows, I'm not seeing the inner highlight on
> the selected tab's curvy strokes:
> 
> http://i.imgur.com/VzZdmro.png
> 
> I re-applied the tab-stroke-start and tab-stroke-end images that Stephen
> linked to, and same result.
> 
> Is this the way it's supposed to look?

No, my WIP patch had two images; one for the border/shadow and one for the highlight. I need to combine them into one.

Comment 4

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

Comment on attachment 753287 [details] [diff] [review]
v.0.9 Use new images from shorlander, fix layering of ::after, remove tab-active-middle-lwtheme.png

Ok, cancelling review until we get the new asset.

Comment 5

[Stephen Horlander [:shorlander]]

Attachment: Updated Tab Images - 01

Combined border and highlight into one.

Comment 6

[Matthew N. [:MattN]]

(In reply to Mike Conley (:mconley) from comment #4)
> Comment on attachment 753287 [details] [diff] [review]
> v.0.9 Use new images from shorlander, fix layering of ::after, remove
> tab-active-middle-lwtheme.png
> 
> Ok, cancelling review until we get the new asset.

Linux and OS X already have the proper images so I was hoping to get review on those platforms.

Comment 7

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

Comment on attachment 753287 [details] [diff] [review]
v.0.9 Use new images from shorlander, fix layering of ::after, remove tab-active-middle-lwtheme.png

Ok, can do.

Comment 8

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

Comment on attachment 753287 [details] [diff] [review]
v.0.9 Use new images from shorlander, fix layering of ::after, remove tab-active-middle-lwtheme.png

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

Code looks good, and testing on Linux and OSX shows the highlighted stroke. Looks good.

Comment 9

[Matthew N. [:MattN]]

Attachment: v.1 Adjusted clip-path, tweaked @2x images, and fix Windows border

Changes since v0.9:
* Clip-path adjusted to work better with updates stroke images and to fix regression with the sliver when flexing tabs on Windows. (with 0.05 overlap).
* Tweaked @2x stroke images by making some pixels transparent in the bottom corner where the highlight intersects the nav-bar so it looks better. 
* Image swap for [e-mail] because it still had the tab fill and thus didn't match the start and end because the texture was missing/different. This is also for consistency with the 1x version
* Adjusted margin-bottom: 1px; of the separator in tabs.inc.css because they were overlapping the highlight instead of just the border.
* browser/themes/windows/browser.css: fixed the position of the #TabsToolbar border as it was being covered up after bug 858089

Stephen: The highlight is not perfect with dark themes (except HiDPI OS X) but the problem is not very noticeable with the default theme on Windows/OS X. The highlight seems to get a bit brighter near the bottom of the stroke. It seems like some pixels need tweaking by hand.  An easy way to test is with a solid black LWT.[1] See above for how I tweaked the @2x strokes. Do you want to try tweak the images some more? I'd like to land this in the next day so could we land with the existing images in the meantime?

https://addons.mozilla.org/en-US/firefox/addon/black-15433/

Comment 10

[Stephen Horlander [:shorlander]]

(In reply to Matthew N. [:MattN] from comment #9)
> Stephen: The highlight is not perfect with dark themes (except HiDPI OS X)
> but the problem is not very noticeable with the default theme on Windows/OS
> X. The highlight seems to get a bit brighter near the bottom of the stroke.
> It seems like some pixels need tweaking by hand.  An easy way to test is
> with a solid black LWT.[1] See above for how I tweaked the @2x strokes. Do
> you want to try tweak the images some more? I'd like to land this in the
> next day so could we land with the existing images in the meantime?

I can tweak them as a followup. Should be a simple image swap once this is landed.

New Tab button seems odd: http://cl.ly/image/1R3r213y2G0f

Comment 11

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

(In reply to Stephen Horlander from comment #10)
> (In reply to Matthew N. [:MattN] from comment #9)
> > Stephen: The highlight is not perfect with dark themes (except HiDPI OS X)
> > but the problem is not very noticeable with the default theme on Windows/OS
> > X. The highlight seems to get a bit brighter near the bottom of the stroke.
> > It seems like some pixels need tweaking by hand.  An easy way to test is
> > with a solid black LWT.[1] See above for how I tweaked the @2x strokes. Do
> > you want to try tweak the images some more? I'd like to land this in the
> > next day so could we land with the existing images in the meantime?
> 
> I can tweak them as a followup. Should be a simple image swap once this is
> landed.
> 
> New Tab button seems odd: http://cl.ly/image/1R3r213y2G0f

That's bug 875894.

Comment 12

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

Comment on attachment 755668 [details] [diff] [review]
v.1 Adjusted clip-path, tweaked @2x images, and fix Windows border

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

This code all looks good to me. If shorlander is happy with the visual result, r=me.

Comment 13

[Matthew N. [:MattN]]

Attachment: v.1.1 Tweak clip-path and fix doubleling up of gradient on Linux

Luna blue:
v.1:   http://cl.ly/image/1L230N1h0x1A
v.1.1: http://people.mozilla.com/~mnoorenberghe/tmp/highlight_luna_blue_tweak_middle.png

Comment 14

[Matthew N. [:MattN]]

https://hg.mozilla.org/projects/ux/rev/2b3f36112908

Comment 15

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/2b3f36112908