Closed
Bug 875326
Opened 11 years ago
Closed 11 years ago
Polish Australis tab stroke inner highlight
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
RESOLVED
FIXED
Firefox 28
People
(Reporter: MattN, Assigned: MattN)
References
Details
(Whiteboard: [Australis:M6])
Attachments
(2 files, 2 obsolete files)
8.80 KB,
application/zip
|
Details | |
55.79 KB,
patch
|
mconley
:
review+
shorlander
:
ui-review+
|
Details | Diff | Splinter Review |
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?
Attachment #753287 -
Flags: review?(mconley)
Flags: needinfo?(shorlander)
Assignee | ||
Updated•11 years ago
|
Status: NEW → ASSIGNED
Comment 1•11 years ago
|
||
(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.
Flags: needinfo?(shorlander)
Comment 2•11 years ago
|
||
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?
Flags: needinfo?(shorlander)
Comment 3•11 years ago
|
||
(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.
Flags: needinfo?(shorlander)
Comment 4•11 years ago
|
||
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.
Attachment #753287 -
Flags: review?(mconley)
Comment 5•11 years ago
|
||
Combined border and highlight into one.
Assignee | ||
Comment 6•11 years ago
|
||
(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•11 years ago
|
||
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.
Attachment #753287 -
Flags: review?(mconley)
Comment 8•11 years ago
|
||
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.
Attachment #753287 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 9•11 years ago
|
||
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 tab-active-middle@2x.png 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/
Attachment #753287 -
Attachment is obsolete: true
Attachment #755668 -
Flags: ui-review?(shorlander)
Attachment #755668 -
Flags: review?(mconley)
Comment 10•11 years ago
|
||
(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•11 years ago
|
||
(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•11 years ago
|
||
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.
Attachment #755668 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 13•11 years ago
|
||
Luna blue: v.1: http://cl.ly/image/1L230N1h0x1A v.1.1: http://people.mozilla.com/~mnoorenberghe/tmp/highlight_luna_blue_tweak_middle.png
Attachment #755668 -
Attachment is obsolete: true
Attachment #755668 -
Flags: ui-review?(shorlander)
Attachment #756757 -
Flags: ui-review?(shorlander)
Attachment #756757 -
Flags: review?(mconley)
Updated•11 years ago
|
Attachment #756757 -
Flags: ui-review?(shorlander) → ui-review+
Updated•11 years ago
|
Attachment #756757 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 14•11 years ago
|
||
https://hg.mozilla.org/projects/ux/rev/2b3f36112908
Flags: in-testsuite-
Whiteboard: [Australis:M6] → [Australis:M6][fixed-in-ux]
Comment 15•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2b3f36112908
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M6][fixed-in-ux] → [Australis:M6]
Target Milestone: --- → Firefox 28
You need to log in
before you can comment on or make changes to this bug.
Description
•