Reader mode menu background color is inconsistent with new toolbar grey

VERIFIED FIXED in Firefox 34

Status

()

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: antlam, Assigned: lucasr)

Tracking

unspecified
Firefox 35
x86
Android
Points:
---

Firefox Tracking Flags

(firefox32 unaffected, firefox33 unaffected, firefox34 verified, firefox35 verified)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 8495390 [details]
Screenshot_2014-09-25-10-30-59.png

As per bug 1055536, our Reader Mode has the same issue with different (not matching) greys. Let's clean this up too.
Flags: needinfo?(lucasr.at.mozilla)
(Assignee)

Comment 1

4 years ago
Created attachment 8495846 [details] [diff] [review]
Use consistent grey tone in reader mode (r=margaret)
(Assignee)

Updated

4 years ago
Attachment #8495846 - Flags: review?(margaret.leibovic)
Flags: needinfo?(lucasr.at.mozilla)
(Assignee)

Comment 2

4 years ago
Created attachment 8495847 [details]
Screenshot
(Assignee)

Comment 3

4 years ago
Fixed the popup divider to have the same color than the button dividers btw.
(Assignee)

Updated

4 years ago
Assignee: nobody → lucasr.at.mozilla
(Reporter)

Comment 4

4 years ago
That reminds me, the dividers look really blue still. I posted a comment about those here: https://bugzilla.mozilla.org/show_bug.cgi?id=1055536#c21 not sure how easy it would be but it's fairly obvious when there are a lot...
Comment on attachment 8495846 [details] [diff] [review]
Use consistent grey tone in reader mode (r=margaret)

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

We'll also need to update the color of the popup arrow:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/images/reader-dropdown-arrow-hdpi.png
http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/images/reader-dropdown-arrow-mdpi.png
http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/images/reader-dropdown-arrow-xhdpi.png

And it seems to me like we should make sure the horizontal dividers also match the vertical dividers:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/aboutReader.css#399
Attachment #8495846 - Flags: review?(margaret.leibovic) → review-
NI to antlam to help update the popup arrow images.
Flags: needinfo?(alam)
(Reporter)

Comment 7

4 years ago
(In reply to :Margaret Leibovic from comment #5)
> Comment on attachment 8495846 [details] [diff] [review]
> Use consistent grey tone in reader mode (r=margaret)
> 
> Review of attachment 8495846 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> We'll also need to update the color of the popup arrow:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/
> images/reader-dropdown-arrow-hdpi.png
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/
> images/reader-dropdown-arrow-mdpi.png
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/
> images/reader-dropdown-arrow-xhdpi.png

Will find this arrow but I have a feeling I'll need to recreate it...

> And it seems to me like we should make sure the horizontal dividers also
> match the vertical dividers:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/
> aboutReader.css#399

YES! :D
(Assignee)

Comment 8

4 years ago
(In reply to :Margaret Leibovic from comment #5)
> Comment on attachment 8495846 [details] [diff] [review]
> Use consistent grey tone in reader mode (r=margaret)
> 
> Review of attachment 8495846 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> We'll also need to update the color of the popup arrow:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/
> images/reader-dropdown-arrow-hdpi.png
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/
> images/reader-dropdown-arrow-mdpi.png
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/
> images/reader-dropdown-arrow-xhdpi.png
> 
> And it seems to me like we should make sure the horizontal dividers also
> match the vertical dividers:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/
> aboutReader.css#399

Nice catches Margaret, thanks!
(Reporter)

Comment 9

4 years ago
Created attachment 8497163 [details]
img_downarrow_newgrey.zip

I performed an educated guess at how these icons would be most useful to you guys.. let's try these! :)

But from some studying of these and looking at how we're implementing them, maybe they're not that useful/necessary anymore... thoughts?
Flags: needinfo?(alam)
(Assignee)

Comment 10

4 years ago
Created attachment 8498217 [details] [diff] [review]
Use consistent grey tone in reader mode (r=margaret)
(Assignee)

Updated

4 years ago
Attachment #8498217 - Flags: review?(margaret.leibovic)
(Assignee)

Updated

4 years ago
Attachment #8495846 - Attachment is obsolete: true
(Assignee)

Comment 11

4 years ago
Created attachment 8498220 [details]
Screenshot
Comment on attachment 8498217 [details] [diff] [review]
Use consistent grey tone in reader mode (r=margaret)

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

Nice.
Attachment #8498217 - Flags: review?(margaret.leibovic) → review+
(Reporter)

Comment 13

4 years ago
(In reply to Lucas Rocha (:lucasr) from comment #11)
> Created attachment 8498220 [details]
> Screenshot

<3
(Assignee)

Comment 15

4 years ago
Comment on attachment 8498217 [details] [diff] [review]
Use consistent grey tone in reader mode (r=margaret)

Approval Request Comment
[Feature/regressing bug #]: bug 1010740
[User impact if declined]: Unpolished UX in reader mode as its color scheme doesn't match the new toolbar style.
[Describe test coverage new/current, TBPL]: Local testing only. Let's bake it in Nightly for a few days and then uplift to Aurora. 
[Risks and why]: Low, just updates a CSS file and a few images. 
[String/UUID change made/needed]: n/a
Attachment #8498217 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/a50ef251a2ab
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
status-firefox32: --- → unaffected
status-firefox33: --- → unaffected
status-firefox34: --- → affected
status-firefox35: --- → fixed
Comment on attachment 8498217 [details] [diff] [review]
Use consistent grey tone in reader mode (r=margaret)

Aurora+
Attachment #8498217 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 19

4 years ago
Verified as fixed om Firefox 34 Beta 4 and latest Aurora, on Asus Transformer Pad TF300T (Android 4.1.1)
Status: RESOLVED → VERIFIED
status-firefox34: fixed → verified
status-firefox35: fixed → verified
You need to log in before you can comment on or make changes to this bug.