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.

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

I increased the line spacing for all views (including the remote tabs panel). See the other attached files. Anthony, what do you think?

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 on attachment 8443793 [details] Portrait Same questions about the font size and line height for you here :)

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

(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

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

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

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

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?

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

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

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 on attachment 8453359 [details] RemoteTabs portrait v4 Looks good! Thanks Michael

Comment on attachment 8453360 [details] Portrait v4 Nice! Looks better. :)

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.

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

Comment on attachment 8455655 [details] Landscape v4 awesome!

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

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.

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 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 on attachment 8455832 [details] [diff] [review] Part 2: Empty private tabs panel visual refinements for tablet Review of attachment 8455832 [details] [diff] [review]: ----------------------------------------------------------------- Good.

Reopening for part 2.

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 on attachment 8455810 [details] [diff] [review] (aurora) Remote tabs tray visual refinements to match trunk Aurora+

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