Fine-tune Lightweight theme colors on new tablet

VERIFIED FIXED in Firefox 36

Status

()

Firefox for Android
Theme and Visual Design
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: mcomella, Assigned: mcomella)

Tracking

unspecified
Firefox 37
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36 verified, firefox37 verified)

Details

Attachments

(8 attachments)

Issues I noticed after bug 1085771 lands:
  * The plus button in the tab strip is light and clashes with light themes
  * The grey text on the unselected tabs clashes with just about everything 
  * (?) Back/forward button state when pressed/focused/etc. might look terrible
  * Pressed tab color in tab strip looks too dark on light themes
Mike, do you think this is v1 material?
Flags: needinfo?(michael.l.comella)
As long as I can get bug 1085771 to land soon, then yes, I think this is v1 material - it'll look pretty amateur if we don't get at least a dark/light swap working.
Flags: needinfo?(michael.l.comella)
Duplicate of this bug: 1110157
Created attachment 8535196 [details]
Part 1 - Dark theme w/ light text color

I wonder if we shouldn't also change the icon colors (about:home favicon, close button, dividers) to a lighter color - maybe after bug 1105546.

Note that we do mischaracterize (imo) some light themes as dark and vice versa, which makes this backfire sometimes.
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Created attachment 8535197 [details] [diff] [review]
Part 1: Use a light text color for tab titles with a dark LWT on new tablet
Attachment #8535197 - Flags: review?(mhaigh)
Created attachment 8535216 [details]
Part 2 - Dark theme w/ alpha pressed state
Created attachment 8535217 [details]
Part 2 - Light theme w/ alpha pressed state
Created attachment 8535218 [details] [diff] [review]
Part 2: Add alpha to pressed state of tabs with LWT on new tablet
Attachment #8535218 - Flags: review?(mhaigh)
Created attachment 8535308 [details]
Part 3 - Light theme, dark add tab button
Anthony, let me know if you have any objections to the screenshots in this bug.
Flags: needinfo?(alam)
Created attachment 8535323 [details] [diff] [review]
Part 3: Change add tab button to a dark color on light themes on new tablet

The added assets are trimaged.

To create the new assets, I ran:

 convert tab_new.png -background #5F6368 -alpha shape <out-file>

where #5F6368 comes from the color I found for drawable-xhdpi/close.png.
Attachment #8535323 - Flags: review?(mhaigh)
Comment on attachment 8535323 [details] [diff] [review]
Part 3: Change add tab button to a dark color on light themes on new tablet

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

Looks good to me
Attachment #8535323 - Flags: review?(mhaigh) → review+
Comment on attachment 8535218 [details] [diff] [review]
Part 2: Add alpha to pressed state of tabs with LWT on new tablet

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

All good.
Attachment #8535218 - Flags: review?(mhaigh) → review+

Updated

3 years ago
Attachment #8535197 - Flags: review?(mhaigh) → review+
(In reply to Michael Comella (:mcomella) from comment #10)
> Anthony, let me know if you have any objections to the screenshots in this
> bug.

Looks good to me. What opacity value are we using BTW? Just so we could reuse some of those numbers later if we need.
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
(In reply to Anthony Lam (:antlam) from comment #14)
> Looks good to me. What opacity value are we using BTW? Just so we could
> reuse some of those numbers later if we need.

For the pressed tabs, 170 / 255.
Flags: needinfo?(michael.l.comella)
Comment on attachment 8535197 [details] [diff] [review]
Part 1: Use a light text color for tab titles with a dark LWT on new tablet

This request applies to parts 1-3, uplift dependency on bug 1085771.

Approval Request Comment
[Feature/regressing bug #]: New tablet release
[User impact if declined]:
  New tablet UI will look unpolished w/ lightweight themes enabled.

[Describe test coverage new/current, TBPL]: None
[Risks and why]: 
  We only touch lightweight theme code so somewhere along the line we could mess up the way lightweight themes work.

[String/UUID change made/needed]: None
Attachment #8535197 - Flags: approval-mozilla-aurora?
status-firefox36: --- → affected
status-firefox37: --- → affected
https://hg.mozilla.org/mozilla-central/rev/f57690535a49
https://hg.mozilla.org/mozilla-central/rev/6ecba299e659
https://hg.mozilla.org/mozilla-central/rev/96e778e6b268
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
status-firefox37: affected → fixed
Attachment #8535197 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in
Firefox for Android 37.0a1 (2014-12-18)
Firefox for Android 36.0a2 (2014-12-18)

Device: Asus Transformer Pad TF300T (Android 4.2.1)
Status: RESOLVED → VERIFIED
status-firefox36: fixed → verified
status-firefox37: fixed → verified

Comment 21

3 years ago
Created attachment 8538447 [details]
device-2014-12-18-142642.png

I can still see the behavior described in Bug 1110157, but only if you tap on the already selected tab; is this intended?
Flags: needinfo?(michael.l.comella)
(In reply to Mihai Pop from comment #21)
> I can still see the behavior described in Bug 1110157, but only if you tap
> on the already selected tab; is this intended?

While I'm not sure I directly consulted :antlam on this one, yes, it is intended.
Flags: needinfo?(michael.l.comella)
You need to log in before you can comment on or make changes to this bug.