Closed
Bug 1073066
Opened 10 years ago
Closed 10 years ago
Reader mode menu background color is inconsistent with new toolbar grey
Categories
(Firefox for Android Graveyard :: Reader View, defect)
Tracking
(firefox32 unaffected, firefox33 unaffected, firefox34 verified, firefox35 verified)
VERIFIED
FIXED
Firefox 35
Tracking | Status | |
---|---|---|
firefox32 | --- | unaffected |
firefox33 | --- | unaffected |
firefox34 | --- | verified |
firefox35 | --- | verified |
People
(Reporter: antlam, Assigned: lucasr)
References
Details
Attachments
(5 files, 1 obsolete file)
174.29 KB,
image/png
|
Details | |
209.75 KB,
image/png
|
Details | |
6.27 KB,
application/zip
|
Details | |
12.82 KB,
patch
|
Margaret
:
review+
lmandel
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
101.71 KB,
image/png
|
Details |
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•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8495846 -
Flags: review?(margaret.leibovic)
Flags: needinfo?(lucasr.at.mozilla)
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Comment 3•10 years ago
|
||
Fixed the popup divider to have the same color than the button dividers btw.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → lucasr.at.mozilla
Reporter | ||
Comment 4•10 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 5•10 years ago
|
||
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-
Reporter | ||
Comment 7•10 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•10 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•10 years ago
|
||
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•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8498217 -
Flags: review?(margaret.leibovic)
Assignee | ||
Updated•10 years ago
|
Attachment #8495846 -
Attachment is obsolete: true
Assignee | ||
Comment 11•10 years ago
|
||
Comment 12•10 years ago
|
||
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•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #11) > Created attachment 8498220 [details] > Screenshot <3
Assignee | ||
Comment 14•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/a50ef251a2ab
Assignee | ||
Comment 15•10 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?
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a50ef251a2ab
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Updated•10 years ago
|
status-firefox32:
--- → unaffected
status-firefox33:
--- → unaffected
status-firefox34:
--- → affected
status-firefox35:
--- → fixed
Comment 17•10 years ago
|
||
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•10 years ago
|
||
Verified as fixed om Firefox 34 Beta 4 and latest Aurora, on Asus Transformer Pad TF300T (Android 4.1.1)
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•