Closed
Bug 1120004
Opened 10 years ago
Closed 10 years ago
Update Reader View controls
Categories
(Firefox for Android Graveyard :: Reader View, defect)
Tracking
(firefox38 verified, firefox39 verified, fennec38+)
RESOLVED
FIXED
Firefox 39
People
(Reporter: antlam, Assigned: Margaret)
References
Details
Attachments
(4 files, 7 obsolete files)
I think that our Reader mode controls could be improved. It seems like a good time to take a look at this now with all the other work going into Reader Mode. The navigational conventions don't resurface anywhere else in our UI or the systems and it seems they're sometimes even overlooked/ not noticeable. E.g. Right now it's not terribly obvious that you can/have to "Add to Reading list" to keep the article in your panel (referred to in bug 1103232)
Also, we could take advantage of Android L (http://www.google.com/design/spec/components/buttons.html#buttons-floating-action-button).
I think an update this to with the goal of offering better UX (much of it doesn't really change, I've only moved the "Add to Reading list" and "Share" button inside), and an updated UI would be really great. My hope is that this makes our awesome controls more obvious, gives the user more of the article, and also aligns better with the rest of our UI.
Currently molding this design for Tablet as well. The first part would be to unify, and then see how we can offer unique improvements on that type of device.
WIP previews attached. :)
Reporter | ||
Comment 1•10 years ago
|
||
Whoops - thought I attached it. Still working on this
Reporter | ||
Comment 2•10 years ago
|
||
Addressing some feedback, I decided to go with something less drastic for this update.
I think we should also loop in bug 998031 at the same time to help with issues of discover-ability.
Mocks for Tablet to follow.
Attachment #8547688 -
Attachment is obsolete: true
Reporter | ||
Comment 3•10 years ago
|
||
Attaching Tablet mock. Not looking to change too much here.
Uses the same menu background image (and therefore shadow) but maybe not the triangle. I'd like to try and move away from using that arrow (and I will mock something similar up for bug 1088220 as well) because it seems almost unnecessary now, and currently aligns weirdly in our UI.
Nothing else should change about our current implementation of the controls and which device & orientation combination gets what.
Assignee | ||
Comment 4•10 years ago
|
||
As part of this (or maybe as its own bug), I think we should also update the share icon to be a paper airplane icon, which will match what's in the desktop mock-ups.
Reporter | ||
Comment 5•10 years ago
|
||
Just realized I forgot the "Aa" button's active state. Also thinking about moving back to +/- that just change the type by a multiple so this design would handle that (taking into consideration iOS too).
Attachment #8548411 -
Attachment is obsolete: true
Reporter | ||
Comment 6•10 years ago
|
||
Updates for Clear Sans, and the "Add to Reading list" icon. Desktop has a + inside a circle but after talking to Maslaney, there are lots of nuances involved in unifying this icon. So, this updated icon at least brings us a step closer (+ is more obvious) for the time being.
Will attach assets later.
Attachment #8556555 -
Attachment is obsolete: true
Assignee | ||
Comment 7•10 years ago
|
||
I'm going to try to get around to this bug this week. It might be hard to land something this week, but if this ends up being mostly CSS changes, it should be upliftable. The main risky bit would be changing the font size controls to +/-, especially if that makes us diverge from the destkop controls, since right now we actually share the same logic for these controls.
Assignee: nobody → margaret.leibovic
tracking-fennec: --- → ?
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #7)
> The main risky bit would be changing the font size
> controls to +/-, especially if that makes us diverge from the destkop
> controls, since right now we actually share the same logic for these
> controls.
Definitely, I'm going to talk to the rest of the team this week to get us on the same page about this.
Updated•10 years ago
|
Status: NEW → ASSIGNED
Hardware: x86 → All
Summary: Update Reader mode controls on Mobile → Update Reader View controls
Reporter | ||
Comment 9•10 years ago
|
||
Spoke with the team and we're going to go with + / - across platforms for V1. Mocks to follow.
Reporter | ||
Comment 10•10 years ago
|
||
Attachment #8549049 -
Attachment is obsolete: true
Attachment #8564203 -
Attachment is obsolete: true
Assignee | ||
Comment 11•10 years ago
|
||
A few questions:
* What's that circle icon at the left of the controls?
* Can we update our share button to be the same as desktop's plane icon?
* You're okay with Serif/Sans-Serif instead of the font names? I thought you had veto-ed that before.
* Is there a desktop bug for updating these controls to the +/- as well?
Flags: needinfo?(alam)
Reporter | ||
Comment 12•10 years ago
|
||
Hey Margaret! sorry about the brevity, wanted to file that before the end of the day and just signed off right after.
(In reply to :Margaret Leibovic from comment #11)
> A few questions:
> * What's that circle icon at the left of the controls?
This is a "Mark as read/unread" button that our iOS side has in their controls and I'm hoping to get some consistency by including this. Especially if we're adding indicators, it'd be nice to give users control over this. I'm thinking we would also leverage our toast's to provide user feedback too.
> * Can we update our share button to be the same as desktop's plane icon?
I know I wanted to get this done as well. But, there seems to be a variety of places in which we use this across all our different platforms. I'm going to start a thread and include the relevant parties to sort this out. In the meantime, we should stick to Android conventions for "share" (3 node icon) and keep using the plane for "Send tab to Firefox on my other devices" as we're doing in the Share overlay.
The plane is also being used for sending and receiving tabs on the iOS side.
> * You're okay with Serif/Sans-Serif instead of the font names? I thought you
> had veto-ed that before.
I did and I still prefer the personal touch of the names.
But, (and we had a long conversation about this with the iOS and Desktop side) since shipping with same fonts brings up a host of other issues, this keeps things a bit more consistent in the controls and causes less confusion in the mean time.
This of course might be different as users study the typefaces but we feel like that's less of a concern than provided a unified Reading List / Reader View experience.
> * Is there a desktop bug for updating these controls to the +/- as well?
I didn't file one, maybe Mike did?
Flags: needinfo?(alam)
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #12)
> Hey Margaret! sorry about the brevity, wanted to file that before the end of
> the day and just signed off right after.
>
> (In reply to :Margaret Leibovic from comment #11)
> > A few questions:
> > * What's that circle icon at the left of the controls?
>
> This is a "Mark as read/unread" button that our iOS side has in their
> controls and I'm hoping to get some consistency by including this.
> Especially if we're adding indicators, it'd be nice to give users control
> over this. I'm thinking we would also leverage our toast's to provide user
> feedback too.
Let's split this into a different bug, since this will require adding a new button and some new logic. Is this button going to exist on desktop as well?
> > * Can we update our share button to be the same as desktop's plane icon?
>
> I know I wanted to get this done as well. But, there seems to be a variety
> of places in which we use this across all our different platforms. I'm going
> to start a thread and include the relevant parties to sort this out. In the
> meantime, we should stick to Android conventions for "share" (3 node icon)
> and keep using the plane for "Send tab to Firefox on my other devices" as
> we're doing in the Share overlay.
>
> The plane is also being used for sending and receiving tabs on the iOS side.
Okay, I'll leave the icon alone for Android, then.
> > * You're okay with Serif/Sans-Serif instead of the font names? I thought you
> > had veto-ed that before.
>
> I did and I still prefer the personal touch of the names.
I'm confused, since your mock-up has Serif/Sans-serif, not the names. Should I implement what's in this mock-up? Or keep the names?
> But, (and we had a long conversation about this with the iOS and Desktop
> side) since shipping with same fonts brings up a host of other issues, this
> keeps things a bit more consistent in the controls and causes less confusion
> in the mean time.
>
> This of course might be different as users study the typefaces but we feel
> like that's less of a concern than provided a unified Reading List / Reader
> View experience.
>
> > * Is there a desktop bug for updating these controls to the +/- as well?
>
> I didn't file one, maybe Mike did?
Okay, I'll be on the lookout for that.
Comment 14•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #13)
> Let's split this into a different bug, since this will require adding a new
> button and some new logic.
FWIW, the backend for this will be done when Bug 1130461 lands. Please mark it as a dep of the new bug!
Reporter | ||
Comment 15•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #13)
> (In reply to Anthony Lam (:antlam) from comment #12)
> > Hey Margaret! sorry about the brevity, wanted to file that before the end of
> > the day and just signed off right after.
> >
> > (In reply to :Margaret Leibovic from comment #11)
> > > A few questions:
> > > * What's that circle icon at the left of the controls?
> >
> > This is a "Mark as read/unread" button that our iOS side has in their
> > controls and I'm hoping to get some consistency by including this.
> > Especially if we're adding indicators, it'd be nice to give users control
> > over this. I'm thinking we would also leverage our toast's to provide user
> > feedback too.
>
> Let's split this into a different bug, since this will require adding a new
> button and some new logic. Is this button going to exist on desktop as well?
>
> > > * Can we update our share button to be the same as desktop's plane icon?
> >
> > I know I wanted to get this done as well. But, there seems to be a variety
> > of places in which we use this across all our different platforms. I'm going
> > to start a thread and include the relevant parties to sort this out. In the
> > meantime, we should stick to Android conventions for "share" (3 node icon)
> > and keep using the plane for "Send tab to Firefox on my other devices" as
> > we're doing in the Share overlay.
> >
> > The plane is also being used for sending and receiving tabs on the iOS side.
>
> Okay, I'll leave the icon alone for Android, then.
>
> > > * You're okay with Serif/Sans-Serif instead of the font names? I thought you
> > > had veto-ed that before.
> >
> > I did and I still prefer the personal touch of the names.
>
> I'm confused, since your mock-up has Serif/Sans-serif, not the names. Should
> I implement what's in this mock-up? Or keep the names?
Serif and Sans-serif please!
> > But, (and we had a long conversation about this with the iOS and Desktop
> > side) since shipping with same fonts brings up a host of other issues, this
> > keeps things a bit more consistent in the controls and causes less confusion
> > in the mean time.
> >
> > This of course might be different as users study the typefaces but we feel
> > like that's less of a concern than provided a unified Reading List / Reader
> > View experience.
> >
> > > * Is there a desktop bug for updating these controls to the +/- as well?
> >
> > I didn't file one, maybe Mike did?
>
> Okay, I'll be on the lookout for that.
NI-ing Maslaney for the Desktop related notes
Flags: needinfo?(mmaslaney)
Assignee | ||
Updated•10 years ago
|
tracking-fennec: ? → 38+
Comment 16•10 years ago
|
||
FWIW we're going with the standard share icon for iOS, not the shareplane.
Comment 17•10 years ago
|
||
Flags: needinfo?(mmaslaney)
Assignee | ||
Comment 18•10 years ago
|
||
I'm going to update the font size controls for both desktop and mobile in bug 1134441, since that will affect the markup/logic of the controls.
As for updating the style for mobile, I'm going to need an orange "Aa" image for the selected state in the controls.
Also, can you specify what the pressed state should look like for these toolbar buttons? Right now a background color is just applied to the rectangular hit area for the buttons.
I also noticed that the serif/sans-serif font sample styles seem to have regressed on Android... I'll have to look into that.
Flags: needinfo?(alam)
Comment 19•10 years ago
|
||
Updated Controls from Desktop: http://invis.io/XT21LURF8
Reporter | ||
Comment 20•10 years ago
|
||
Here are icons in SVG
- Aa
- Aa Active
- Read
- Unread
- Add to RL
- Trash (from RL)
- Share
Flags: needinfo?(alam)
Reporter | ||
Comment 21•10 years ago
|
||
As for the pressed state, we can just keep it the same as our menu items right now where the background changes to #D7D7DC. I have ideas for something more polished but it needs more work I think (on my end).
Reporter | ||
Comment 22•10 years ago
|
||
Non- SVG versions for the controls
Updated•10 years ago
|
QA Contact: teodora.vermesan
Reporter | ||
Comment 23•10 years ago
|
||
Color specs:
---
Menu background matte: #EBEBF0
Menu item pressed state: #D7D7DC
Above-menu divider color: #D7D9DB
Typeface active: #222222
Typeface inactive: #AFB1B3
Typeface font-size: 24px ? (converted from 24sp)
Dark-theme button matte: #222222
Dark-theme text: Roboto, regular, 14px? (converted from sp) #EEEEEE
Dark-theme border: 4px rounded corners, 1px solid #BFBFBF
Auto button matte: #EBEBF0
Auto button text: Roboto, regular, 14px? (converted from 14sp) #222222
Auto button border: 4px rounded corners, 1px solid #BFBFBF
Light-theme button matte: #FFFFFF
Light-theme button text: Roboto, regular, 14px? (converted from 14sp) #222222
Light-theme button border: 4px rounded corners, 1px solid #BFBFBF
Dark-theme/ Auto/ Light-theme button active: Border 2px #FF9500
Assignee | ||
Comment 24•10 years ago
|
||
I tried updating the toolbar button icons, but they're all different sizes here, which is causing issues. Currently we use 30x24dpi for those icons. Could you update your new icons to match that size?
Flags: needinfo?(alam)
Assignee | ||
Comment 25•10 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #23)
> Dark-theme button matte: #222222
Currently our dark theme uses #000000. Should I update it to use #222222 instead?
Assignee | ||
Updated•10 years ago
|
status-firefox38:
--- → affected
Assignee | ||
Comment 26•10 years ago
|
||
/r/4467 - Bug 1120004 - Update styles of Reader View controls on mobile. r=bnicholson
Pull down this commit:
hg pull review -r 2776c8636dd4fc9e772d87a596f5c2a7acc6a393
Attachment #8570803 -
Flags: review?(bnicholson)
Assignee | ||
Comment 27•10 years ago
|
||
Comment on attachment 8570803 [details]
MozReview Request: bz://1120004/margaret
This is still a WIP. Things I need to fix:
* Toolbar icons are improperly scaled
* Try to add "Aa" between font size buttons
* Tablet support!
Here are screenshots so far:
http://cl.ly/image/3m2I1n2J2128
http://cl.ly/image/2j2Y3a280C3z
This switch to using proper buttons also gives us keyboard navigation for free on desktop, which is pretty sweet. And it fixes bug 1136245.
Attachment #8570803 -
Flags: review?(bnicholson) → feedback?(bnicholson)
Reporter | ||
Comment 28•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #25)
> (In reply to Anthony Lam (:antlam) from comment #23)
>
> > Dark-theme button matte: #222222
>
> Currently our dark theme uses #000000. Should I update it to use #222222
> instead?
Yes please! That was a conscious decision we made in our hack week and I think it was spec'd for Desktop too.
Reporter | ||
Comment 29•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #24)
> I tried updating the toolbar button icons, but they're all different sizes
> here, which is causing issues. Currently we use 30x24dpi for those icons.
> Could you update your new icons to match that size?
Ah, I know why! I forgot to mention, my design makes the controls along the bottom 56dp tall (in accordance with Android L guidelines) rather than 48 dp (our current toolbar height). It was a conscious decision at the time since I felt our controls were a bit small and harder to notice (especially on larger phones).
After playing with this for a while now, this does increase tab-ability for us although it strays from our current 48 dp height. Can we try the new 56 dp height?
I mocked this up in framer and tested it on my N5, seemed quite comfortable. (56dp = 168px tall, then plus the divider).
Flags: needinfo?(alam) → needinfo?(margaret.leibovic)
Comment 32•10 years ago
|
||
Comment on attachment 8570803 [details]
MozReview Request: bz://1120004/margaret
Looks fine to me so far.
Attachment #8570803 -
Flags: feedback?(bnicholson) → feedback+
Assignee | ||
Comment 33•10 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #29)
> (In reply to :Margaret Leibovic from comment #24)
> > I tried updating the toolbar button icons, but they're all different sizes
> > here, which is causing issues. Currently we use 30x24dpi for those icons.
> > Could you update your new icons to match that size?
>
> Ah, I know why! I forgot to mention, my design makes the controls along the
> bottom 56dp tall (in accordance with Android L guidelines) rather than 48 dp
> (our current toolbar height). It was a conscious decision at the time since
> I felt our controls were a bit small and harder to notice (especially on
> larger phones).
>
> After playing with this for a while now, this does increase tab-ability for
> us although it strays from our current 48 dp height. Can we try the new 56
> dp height?
>
> I mocked this up in framer and tested it on my N5, seemed quite comfortable.
> (56dp = 168px tall, then plus the divider).
Okay, I can try this out, but could you make the icons all the same dimensions?
The way that we do hdpi/xhdpi with CSS is that we explicitly set the pixel size of the image to the mdpi value, then include the higher resolution icons for other resolutions, and it's easier if I can just set a generic width/height for all of the icons.
Currently, all the icons in the toolbar are 30x24dp, and we can increase that, but it would make my life a lot easier if we increased them all consistently.
Flags: needinfo?(alam)
Reporter | ||
Comment 34•10 years ago
|
||
Try these! Every icon (MDPI) is 30dp x 28dp. I've included the others just in case.
Attachment #8568837 -
Attachment is obsolete: true
Flags: needinfo?(alam) → needinfo?(margaret.leibovic)
Assignee | ||
Updated•10 years ago
|
Attachment #8570803 -
Flags: feedback+ → review?(bnicholson)
Assignee | ||
Comment 35•10 years ago
|
||
Comment on attachment 8570803 [details]
MozReview Request: bz://1120004/margaret
/r/4467 - Bug 1120004 - Update styles of Reader View controls on mobile. r=bnicholson
Pull down this commit:
hg pull review -r fb5a79de7324b1766decccd3c216e02ad3eab3e3
Assignee | ||
Comment 36•10 years ago
|
||
FYI, I pngcrush-ed all the new icons.
Here are screenshots:
http://cl.ly/image/1t0m2D0g2q0c
http://cl.ly/image/1I1S2a1Q1y2K
http://cl.ly/image/1m2c382e3V0Q
Flags: needinfo?(margaret.leibovic)
Assignee | ||
Comment 37•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #36)
> http://cl.ly/image/1I1S2a1Q1y2K
(This tablet one is a bit out of date, and the +/- buttons are closer to the center in the final version of my patch)
Comment 38•10 years ago
|
||
Comment on attachment 8570803 [details]
MozReview Request: bz://1120004/margaret
https://reviewboard.mozilla.org/r/4465/#review3755
Ship It!
Attachment #8570803 -
Flags: review?(bnicholson) → review+
Assignee | ||
Comment 39•10 years ago
|
||
Comment 40•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Comment 41•10 years ago
|
||
Tested using:
Build: Firefox for Android 39.0a1 (2015-03-05)
Phone: LG Nexus 4 (Android 4.4)
Tablet: Sony Zperia Z2 (Android 4.4.4)
Reader view controls for tablet:
-portrait: http://i.imgur.com/iH82HM6.png
-landscape: http://i.imgur.com/h1vnNNd.png
Reader view controls for phone:
-portrait: http://i.imgur.com/CVhXyHi.png
-landscape: http://i.imgur.com/lVoidyX.png
Assignee | ||
Comment 42•10 years ago
|
||
Comment 43•10 years ago
|
||
Tested using:
Build: Firefox for Android 38.0a2 (2015-03-06)
Phone: Alcatel One Touch (Android 4.1.2)
Tablet: Sony Zperia Z2 (Android 4.4.4)
Reader view controls for tablet:
-portrait: http://i.imgur.com/efJNtpX.png
-landscape: http://i.imgur.com/HJaKbY0.png
Reader view controls for phone:
-portrait: http://i.imgur.com/vfcv0Sg.png
-landscape: http://i.imgur.com/q5QKrN0.png
Comment 44•10 years ago
|
||
Based on the screenshots from comment 41 and comment 43, I will mark status-firefox38 and 39 as verified. I will leave the overall status as resolved fixed due to bug blocks and dependencies.
Assignee | ||
Comment 45•9 years ago
|
||
Attachment #8570803 -
Attachment is obsolete: true
Attachment #8619098 -
Flags: review+
Assignee | ||
Comment 46•9 years ago
|
||
Updated•4 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
•