1021356 - Refine visuals in empty private tabs screen

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Some desired changes, quoted via bug 1002303 comment 13:
  * The line-height of the copy on the left definitely needs to be increased
  * I would also like to move the "active" tab a bit further down to open up these horizontal screens but I feel like that is going to open up a nest of other problems.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Don't forget to check for small screen and l10n issues, like bug 1007442.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Empty private tabs panel visual refinements.

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Portrait

I increased the line spacing for all views (including the remote tabs panel). See the other attached files.

Anthony, what do you think?

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Landscape

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: RemoteTabs portrait

Comment 6

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

Comment on attachment 8443796 [details]
RemoteTabs portrait

What font are you using here Michael? It seems a bit off to me. It might help if I knew what font size and line height measurements you were using :)

Comment 7

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

Comment on attachment 8443793 [details]
Portrait

Same questions about the font size and line height for you here :)

Comment 8

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

Comment on attachment 8443795 [details]
Landscape

Line height seems excessive here especially if I try to imagine what it would look like if the length of this copy extends even just a little. Can we also try to center the "Want to learn more?" (equal padding top and bottom)?

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Landscape w/ bounds

(In reply to Anthony Lam (:antlam) from comment #6)
> Comment on attachment 8443796 [details]
> RemoteTabs portrait
> 
> What font are you using here Michael?

It should be the Android default, Roboto [1]. I'm not sure which style (thin, light, regular, etc.). this is, however.

> It might help if I knew what font size and line height measurements you were using :)

The font size is 16dp, or 16 density-independent pixels. For a more thorough explanation, see [2].

As for line height, that is declared through a multiplier which is currently 1.2. I can get the exact pixel measurements if you need them.

(In reply to Anthony Lam (:antlam) from comment #7)
> Same questions about the font size and line height for you here :)

They currently share the same styles and so the values should be equivalent.

(In reply to Anthony Lam (:antlam) from comment #8)
> Line height seems excessive here especially if I try to imagine what it
> would look like if the length of this copy extends even just a little.

What do you mean by the length of this copy?
> Can we also try to center the "Want to learn more?"
> (equal padding top and bottom)?

The padding is actually equal on the top and the bottom, to the tab edge, but it doesn't align with the center of the text to the left, which probably gives it this illusion. See the attached file for screenshot with the view bounds.

What do you think I should do here?

[1]: https://developer.android.com/design/style/typography.html
[2]: https://developer.android.com/guide/practices/screens_support.html#terms

Comment 10

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

(In reply to Michael Comella (:mcomella) from comment #9)
> It should be the Android default, Roboto [1]. I'm not sure which style
> (thin, light, regular, etc.). this is, however.

We should be using Regular.

> The font size is 16dp, or 16 density-independent pixels. For a more thorough
> explanation, see [2].

Body font and header font should be different in size. Let's try 16 dp for the header and call-to-action button text and use 14 dp for everything else? I checked over my older files and unfortunately I made them in a pretty old school manner so I can't give you those measurements as they wouldn't round nicely.

Let's try these :)

> As for line height, that is declared through a multiplier which is currently
> 1.2. I can get the exact pixel measurements if you need them.

Can we use 1.5 as the multiplier? That usually makes for the best readability wise but we might still have to tweak it as we go.

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Portrait v2

Anthony, I made the changes - what do you think?

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: RemoteTabs portrait v2

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

For the record, the headers and call-to-action button font size was 20dp in the previous revision.

Comment 14

[Lucas Rocha (:lucasr)]

Comment on attachment 8443792 [details] [diff] [review]
Empty private tabs panel visual refinements.

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

I assume this is to avoid truncating the view's content on smaller screens or while in landscape orientation?

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Lucas Rocha (:lucasr) from comment #14)
> I assume this is to avoid truncating the view's content on smaller screens
> or while in landscape orientation?

Primarily, yes.

Comment 16

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

Comment on attachment 8449062 [details]
RemoteTabs portrait v2

The bounding width of the text box where the "sign in to sync your tabs..." seems awfully wide. Can we make that the width of the button?

Comment 17

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

Comment on attachment 8449061 [details]
Portrait v2

Same thing WRT the width of the text field.

Can we also equate the padding that exists above and below the "Private browsing" title?

Thanks Michael!

Comment 18

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: RemoteTabs portrait v3

Comment 19

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Portrait v3

Comment 20

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Landscape v2

Comment 21

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

Comment on attachment 8451913 [details]
Landscape v2

Yeah, that overflow is a bit of an issue.

What're our options when it comes to pushing the "active tab" down there further down? I feel like this may have larger implications.

Out of curiosity, does using a ratio of 1.35 for the line-height help this at all here?

Comment 22

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

Comment on attachment 8451912 [details]
Portrait v3

This is looking good. I am on the fence ATM when it comes to the visual relationship between the header and the body. I feel like font size difference may not be enough to create a hierarchy here. 

Can we try 18dp for the header? Sorry Michael, I realize I've previously asked for 16dp. The body can remain at 14dp. I think this would really drive it home. :)

Comment 23

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: RemoteTabs portrait v4

The styles are shared between panels, so here's the updated RemoteTabs panel.

Comment 24

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Portrait v4

Comment 25

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Landscape v3

I'm unfortunately not sure how to lower the currently open tab, but we can consider that in a followup bug, if you'd like.

Comment 26

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

Comment on attachment 8453359 [details]
RemoteTabs portrait v4

Looks good! Thanks Michael

Comment 27

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

Comment on attachment 8453360 [details]
Portrait v4

Nice! Looks better. :)

Comment 28

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

Comment on attachment 8453362 [details]
Landscape v3

This is getting there, I think if we just equate the padding above and below the copy on the left, we can wrap this up for now.

Comment 29

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Landscape v4

Comment 30

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Empty private tabs panel visual refinements

Updated styles as per Anthony's comments; Functionally equivalent.

Comment 31

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

Comment on attachment 8455655 [details]
Landscape v4

awesome!

Comment 32

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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

Comment 33

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Forgot to include r=lucasr on the patch in comment 32.

Comment 34

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: (aurora) Remote tabs tray visual refinements to match trunk

Back-port the changes to Aurora, only for the remote tabs panel. Note that I
forgot to do tablets, so this patch includes the tablet changes (okayed by
antlam in-person). I'll make a part 2 to deal with this on trunk.

Comment 35

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Part 2: Empty private tabs panel visual refinements for tablet

There were a few extra changes over the patch in comment 34 because we now have to work with the private tabs panel which shares the same styles. Note that this patch does not fix bug 1037740.

Comment 36

[Lucas Rocha (:lucasr)]

Comment on attachment 8455810 [details] [diff] [review]
(aurora) Remote tabs tray visual refinements to match trunk

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

As long as antlam approves :-)

Comment 37

[Lucas Rocha (:lucasr)]

Comment on attachment 8455832 [details] [diff] [review]
Part 2: Empty private tabs panel visual refinements for tablet

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

Good.

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0bc4b2de1f95

Comment 39

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Reopening for part 2.

Comment 40

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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

Comment 41

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8455810 [details] [diff] [review]
(aurora) Remote tabs tray visual refinements to match trunk

Approval Request Comment
[Feature/regressing bug #]:
  Initially implemented the remote tabs tray in bug 958889, bug 1014999 (tablet), and various other refinement bugs. The style of these trays changed again with the creation of the private tabs tray (this bug) - this patch uplifts these stylistic changes.

[User impact if declined]:
  Users will see one version of the remote tabs tray in 32, and a visual refresh in 33.

[Describe test coverage new/current, TBPL]:
  None, though these are purely style changes.

[Risks and why]:
  We exclusively change the styles used by the remote tabs tray so in the worst case, the remote tabs tray appeareance is broken, and the tray can become completely useless.

[String/UUID change made/needed]: None

Comment 42

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8455810 [details] [diff] [review]
(aurora) Remote tabs tray visual refinements to match trunk

Aurora+

Comment 43

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 44

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3c3f3fa6b3cc

Comment 45

[Flaviu Cos]

Verified as fixed in build 33 beta 2;
Devices:
- Google Nexus 7 (Android 4.4.4);
- Samsung Galaxy R (Android 2.3.4).