1120004 - Update Reader View controls

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Reader View, defect)

Description

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

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. :)

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_controlsupdate_mob2.png

Whoops - thought I attached it. Still working on this

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_controlsupdate_mob4.png

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.

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_controlsupdate_tab4.png

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.

Comment 4

[:Margaret Leibovic]

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.

Comment 5

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_controlsupdate_mob5.png

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).

Comment 6

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_controlsupdate_mob6.png

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.

Comment 7

[:Margaret Leibovic]

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.

Comment 8

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(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.

Comment 9

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Spoke with the team and we're going to go with + / - across platforms for V1. Mocks to follow.

Comment 10

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_controlsupdate_mob7.png

Comment 11

[:Margaret Leibovic]

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?

Comment 12

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

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?

Comment 13

[:Margaret Leibovic]

(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

[Richard Newman [:rnewman]]

(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!

Comment 15

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(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

Comment 16

[Robin Andersen [:tecgirl]]

FWIW we're going with the standard share icon for iOS, not the shareplane.

Comment 17

[Michael Maslaney [:mmaslaney] ([e-mail])]

Bug 1134441

Comment 18

[:Margaret Leibovic]

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.

Comment 19

[Michael Maslaney [:mmaslaney] ([e-mail])]

Updated Controls from Desktop: http://invis.io/XT21LURF8

Comment 20

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: icon_RView_SVG.zip

Here are icons in SVG
 - Aa
 - Aa Active
 - Read
 - Unread
 - Add to RL
 - Trash (from RL)
 - Share

Comment 21

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

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).

Comment 22

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: icon_RView.zip

Non- SVG versions for the controls

Comment 23

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

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

Comment 24

[:Margaret Leibovic]

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?

Comment 25

[:Margaret Leibovic]

(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?

Comment 26

[:Margaret Leibovic]

Attachment: 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 2776c8636dd4fc9e772d87a596f5c2a7acc6a393

Comment 27

[:Margaret Leibovic]

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.

Comment 28

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(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.

Comment 29

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(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).

Comment 32

[Brian Nicholson (:bnicholson)]

Comment on attachment 8570803 [details]
MozReview Request: bz://1120004/margaret

Looks fine to me so far.

Comment 33

[:Margaret Leibovic]

(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.

Comment 34

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: icon_Padded.zip

Try these! Every icon (MDPI) is 30dp x 28dp. I've included the others just in case.

Comment 35

[:Margaret Leibovic]

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

Comment 36

[:Margaret Leibovic]

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

Comment 37

[:Margaret Leibovic]

(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

[Brian Nicholson (:bnicholson)]

Comment on attachment 8570803 [details]
MozReview Request: bz://1120004/margaret

https://reviewboard.mozilla.org/r/4465/#review3755

Ship It!

Comment 39

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/d819aa9d47f6

Comment 40

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d819aa9d47f6

Comment 41

[Teodora Vermesan (:TeoVermesan)]

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

Comment 42

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f03e25d599eb

Comment 43

[Teodora Vermesan (:TeoVermesan)]

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

[Teodora Vermesan (:TeoVermesan)]

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.

Comment 46

[:Margaret Leibovic]

Attachment: MozReview Request: Bug 1120004 - Update styles of Reader View controls on mobile. r=bnicholson