[Photon] Add vibrancy to tabstrip in Light theme

VERIFIED FIXED in Firefox 58

Status

()

Firefox
Theme
P1
normal
VERIFIED FIXED
4 months ago
6 days ago

People

(Reporter: nhnt11, Assigned: nhnt11)

Tracking

Trunk
Firefox 58
Unspecified
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox58 verified)

Details

(Whiteboard: [reserve-photon-visual][p3])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

Comment hidden (empty)

Updated

4 months ago
Flags: qe-verify?
Whiteboard: [photon-visual] → [photon-visual] [triage]

Updated

4 months ago
Priority: -- → P3
Whiteboard: [photon-visual] [triage] → [reserve-photon-visual]

Updated

3 months ago
Flags: qe-verify? → qe-verify+
Whiteboard: [reserve-photon-visual] → [reserve-photon-visual][p3]

Updated

3 months ago
QA Contact: brindusa.tot

Updated

3 months ago
Priority: P3 → P4

Updated

3 months ago
Duplicate of this bug: 1234918

Updated

2 months ago
Priority: P4 → P3
Comment hidden (mozreview-request)

Comment 3

24 days ago
mozreview-review
Comment on attachment 8922335 [details]
Bug 1386721 - Add vibrancy to tabstrip in light theme.

https://reviewboard.mozilla.org/r/193384/#review198590

What about the dark theme?
Attachment #8922335 - Flags: review?(dao+bmo)

Updated

24 days ago
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Priority: P3 → P1
(Assignee)

Comment 4

24 days ago
Created attachment 8922338 [details]
Dark theme screenshot

(In reply to Dão Gottwald [::dao] from comment #3)
> Comment on attachment 8922335 [details]
> Bug 1386721 - Add vibrancy to tabstrip in light theme.
> 
> https://reviewboard.mozilla.org/r/193384/#review198590
> 
> What about the dark theme?

I was just about to comment about this. :P

I don't think we should do this for the dark theme without further discussion. Personally, I think it looks ugly and also kinda goes against the philosophy of the dark theme. See screenshots.
Looks fine to me. Please consult shorlander.
(Assignee)

Comment 6

24 days ago
12:30:21 AM GMT+5:30 - shorlander: Yeah I think the problem with the dark theme is that foreground can end up [darker] than the [background].
12:30:41 AM GMT+5:30 - shorlander: Can we try it on just the light theme?

So, let's land this patch as is and update the bug title?
Flags: needinfo?(dao+bmo)
(In reply to Nihanth Subramanya [:nhnt11] from comment #6)
> 12:30:21 AM GMT+5:30 - shorlander: Yeah I think the problem with the dark
> theme is that foreground can end up [darker] than the [background].

I'm not sure what this means. What's foreground and background here? Navbar + selected tab vs. background tabs? Background tabs are always darker in the dark theme on Windows and Mac...
Flags: needinfo?(dao+bmo)

Comment 8

23 days ago
> I don't think we should do this for the dark theme without further discussion. Personally, I think it looks ugly

I would like to understand this statement. The dark theme looks ugly with vibrance so it shouldn't be implemented, you say. But the default theme has also a dark tab strip and vibrancy was implemented for the default theme. I understand that not everyone like the vibrancy effect (I am also not the biggest fan) but I don't understand why it should be there in the default theme and not in the dark theme because both has a similar tab strip on macOS. I would also understand that it's maybe desirable to have a theme without vibrancy but then I don't understand why the light theme should have vibrancy and the dark theme should not. Maybe an option for enabling/disabling the vibrancy would be great, but this would be another request and I don't think the light theme and the dark theme should be treated differently. ;)
(Assignee)

Comment 9

23 days ago
(In reply to Sören Hentzschel from comment #8)
> > I don't think we should do this for the dark theme without further discussion. Personally, I think it looks ugly
> 
> I would like to understand this statement. The dark theme looks ugly with
> vibrance so it shouldn't be implemented, you say. But the default theme has
> also a dark tab strip and vibrancy was implemented for the default theme. I
> understand that not everyone like the vibrancy effect (I am also not the
> biggest fan) but I don't understand why it should be there in the default
> theme and not in the dark theme because both has a similar tab strip on
> macOS. I would also understand that it's maybe desirable to have a theme
> without vibrancy but then I don't understand why the light theme should have
> vibrancy and the dark theme should not. Maybe an option for
> enabling/disabling the vibrancy would be great, but this would be another
> request and I don't think the light theme and the dark theme should be
> treated differently. ;)

I don't think the background color of the nav toolbar and selected tab in the dark theme goes well with vibrancy. This is my personal subjective opinion and hence my desire for further discussion. :)

For me, one way to put it would be that vibrancy can mean a tinted background (see attachment 8922338 [details]) which clashes with the dark background of the tabs/navbar (but I don't think it clashes with the bright background used in the default and light themes).

(In reply to Dão Gottwald [::dao] from comment #7)
> (In reply to Nihanth Subramanya [:nhnt11] from comment #6)
> > 12:30:21 AM GMT+5:30 - shorlander: Yeah I think the problem with the dark
> > theme is that foreground can end up [darker] than the [background].
> 
> I'm not sure what this means. What's foreground and background here? Navbar
> + selected tab vs. background tabs? Background tabs are always darker in the
> dark theme on Windows and Mac...

Stephen, could you clarify please?
Flags: needinfo?(shorlander)
Sorry, what I meant to say was:

That the Background Tabs & Tabstrip with vibrancy can end-up brighter than the Foreground Tabs & Toolbar. Which visually messes with the z-order we want to establish.

The light theme doesn't seem to have the problem and I think it looks nice there. So we should leave the tabstrip fully opaque for the dark theme.
Flags: needinfo?(shorlander)
(In reply to Stephen Horlander [:shorlander] from comment #10)
> Sorry, what I meant to say was:
> 
> That the Background Tabs & Tabstrip with vibrancy can end-up brighter than
> the Foreground Tabs & Toolbar. Which visually messes with the z-order we
> want to establish.

This is already the case atm with the dark theme without vibrancy...
(Assignee)

Comment 12

23 days ago
(In reply to Nihanth Subramanya [:nhnt11] from comment #4)
> Created attachment 8922338 [details]
> Dark theme screenshot
> 
> (In reply to Dão Gottwald [::dao] from comment #3)
> > Comment on attachment 8922335 [details]
> > Bug 1386721 - Add vibrancy to tabstrip in light theme.
> > 
> > https://reviewboard.mozilla.org/r/193384/#review198590
> > 
> > What about the dark theme?
> 
> I was just about to comment about this. :P
> 
> I don't think we should do this for the dark theme without further
> discussion. Personally, I think it looks ugly and also kinda goes against
> the philosophy of the dark theme. See screenshots.

Sorry, I wasn't clear about what this screenshot is showing: both cases shown are with vibrancy enabled. In the first one, there's something light behind Firefox, and in the second one, my bright wallpaper is behind Firefox.

Comment 13

23 days ago
mozreview-review
Comment on attachment 8922335 [details]
Bug 1386721 - Add vibrancy to tabstrip in light theme.

https://reviewboard.mozilla.org/r/193384/#review199034
Attachment #8922335 - Flags: review+

Updated

23 days ago
Summary: [Photon] Add vibrancy to tabstrip in dark and light themes → [Photon] Add vibrancy to tabstrip in light themes

Comment 14

23 days ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b4f2482fac1f
Add vibrancy to tabstrip in light theme. r=dao
https://hg.mozilla.org/mozilla-central/rev/b4f2482fac1f
Status: ASSIGNED → RESOLVED
Last Resolved: 23 days ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
(Assignee)

Updated

23 days ago
Depends on: 1412430

Updated

18 days ago
Summary: [Photon] Add vibrancy to tabstrip in light themes → [Photon] Add vibrancy to tabstrip in Light theme
I verified this on Mac OS X 10.13 with FF Nightly 58.0a1(2017-11-07) and I can confirm that the vibrancy is added to tabstrip in Light theme.
Status: RESOLVED → VERIFIED
status-firefox58: fixed → verified
You need to log in before you can comment on or make changes to this bug.