Refine visuals in empty private tabs screen

RESOLVED FIXED in Firefox 32

Status

()

Firefox for Android
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mcomella, Assigned: mcomella)

Tracking

unspecified
Firefox 33
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox30 unaffected, firefox31 unaffected, firefox32 fixed, firefox33 verified)

Details

Attachments

(7 attachments, 10 obsolete attachments)

114.31 KB, image/png
Details
68.62 KB, image/png
antlam
: feedback+
Details
79.34 KB, image/png
antlam
: feedback+
Details
72.39 KB, image/png
antlam
: feedback+
Details
10.86 KB, patch
lucasr
: review+
Details | Diff | Splinter Review
8.33 KB, patch
lucasr
: review+
Details | Diff | Splinter Review
8.90 KB, patch
lucasr
: review+
Details | Diff | Splinter Review
(Assignee)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
Don't forget to check for small screen and l10n issues, like bug 1007442.
(Assignee)

Comment 2

3 years ago
Created attachment 8443792 [details] [diff] [review]
Empty private tabs panel visual refinements.
Attachment #8443792 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Updated

3 years ago
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
(Assignee)

Comment 3

3 years ago
Created attachment 8443793 [details]
Portrait

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

Anthony, what do you think?
Attachment #8443793 - Flags: feedback?(alam)
(Assignee)

Comment 4

3 years ago
Created attachment 8443795 [details]
Landscape
Attachment #8443795 - Flags: feedback?(alam)
(Assignee)

Comment 5

3 years ago
Created attachment 8443796 [details]
RemoteTabs portrait
Attachment #8443796 - Flags: feedback?(alam)
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)?
Attachment #8443795 - Flags: feedback?(alam) → feedback-

Updated

3 years ago
Attachment #8443796 - Flags: feedback?(alam) → feedback-

Updated

3 years ago
Attachment #8443793 - Flags: feedback?(alam) → feedback-
(Assignee)

Comment 9

3 years ago
Created attachment 8444863 [details]
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
Flags: needinfo?(alam)
(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.
Flags: needinfo?(alam)
(Assignee)

Comment 11

3 years ago
Created attachment 8449061 [details]
Portrait v2

Anthony, I made the changes - what do you think?
Attachment #8443793 - Attachment is obsolete: true
Attachment #8449061 - Flags: feedback?(alam)
(Assignee)

Comment 12

3 years ago
Created attachment 8449062 [details]
RemoteTabs portrait v2
Attachment #8443796 - Attachment is obsolete: true
Attachment #8449062 - Flags: feedback?(alam)
(Assignee)

Comment 13

3 years ago
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?
Attachment #8443792 - Flags: review?(lucasr.at.mozilla) → review+
(Assignee)

Comment 15

3 years ago
(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?
Attachment #8449062 - Flags: feedback?(alam) → feedback-
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!
Attachment #8449061 - Flags: feedback?(alam) → feedback-
(Assignee)

Comment 18

3 years ago
Created attachment 8451909 [details]
RemoteTabs portrait v3
Attachment #8449062 - Attachment is obsolete: true
Attachment #8451909 - Flags: feedback?(alam)
(Assignee)

Comment 19

3 years ago
Created attachment 8451912 [details]
Portrait v3
Attachment #8449061 - Attachment is obsolete: true
Attachment #8451912 - Flags: feedback?(alam)
(Assignee)

Comment 20

3 years ago
Created attachment 8451913 [details]
Landscape v2
Attachment #8443795 - Attachment is obsolete: true
Attachment #8451913 - Flags: feedback?(alam)

Updated

3 years ago
Attachment #8451909 - Flags: feedback?(alam) → feedback+
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?
Attachment #8451913 - Flags: feedback?(alam) → feedback-
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. :)
Attachment #8451912 - Flags: feedback?(alam) → feedback-
(Assignee)

Comment 23

3 years ago
Created attachment 8453359 [details]
RemoteTabs portrait v4

The styles are shared between panels, so here's the updated RemoteTabs panel.
Attachment #8451909 - Attachment is obsolete: true
Attachment #8453359 - Flags: feedback?(alam)
(Assignee)

Comment 24

3 years ago
Created attachment 8453360 [details]
Portrait v4
Attachment #8451912 - Attachment is obsolete: true
Attachment #8453360 - Flags: feedback?(alam)
(Assignee)

Comment 25

3 years ago
Created attachment 8453362 [details]
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.
Attachment #8451913 - Attachment is obsolete: true
Attachment #8453362 - Flags: feedback?(alam)
(Assignee)

Updated

3 years ago
Blocks: 1037740
Comment on attachment 8453359 [details]
RemoteTabs portrait v4

Looks good! Thanks Michael
Attachment #8453359 - Flags: feedback?(alam) → feedback+
Comment on attachment 8453360 [details]
Portrait v4

Nice! Looks better. :)
Attachment #8453360 - Flags: feedback?(alam) → feedback+
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.
Attachment #8453362 - Flags: feedback?(alam) → feedback-
(Assignee)

Comment 29

3 years ago
Created attachment 8455655 [details]
Landscape v4
Attachment #8453362 - Attachment is obsolete: true
Attachment #8455655 - Flags: feedback?(alam)
(Assignee)

Comment 30

3 years ago
Created attachment 8455657 [details] [diff] [review]
Empty private tabs panel visual refinements

Updated styles as per Anthony's comments; Functionally equivalent.
Attachment #8455657 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Updated

3 years ago
Attachment #8443792 - Attachment is obsolete: true
Comment on attachment 8455655 [details]
Landscape v4

awesome!
Attachment #8455655 - Flags: feedback?(alam) → feedback+

Updated

3 years ago
Attachment #8455657 - Flags: review?(lucasr.at.mozilla) → review+
(Assignee)

Comment 32

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/0bc4b2de1f95
(Assignee)

Comment 33

3 years ago
Forgot to include r=lucasr on the patch in comment 32.
(Assignee)

Comment 34

3 years ago
Created attachment 8455810 [details] [diff] [review]
(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.
Attachment #8455810 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 35

3 years ago
Created attachment 8455832 [details] [diff] [review]
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.
Attachment #8455832 - Flags: review?(lucasr.at.mozilla)
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 :-)
Attachment #8455810 - Flags: review?(lucasr.at.mozilla) → review+
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.
Attachment #8455832 - Flags: review?(lucasr.at.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/0bc4b2de1f95
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
(Assignee)

Comment 39

3 years ago
Reopening for part 2.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 40

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/e9d78c3d2eb5
(Assignee)

Comment 41

3 years ago
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
Attachment #8455810 - Flags: approval-mozilla-aurora?
Comment on attachment 8455810 [details] [diff] [review]
(aurora) Remote tabs tray visual refinements to match trunk

Aurora+
Attachment #8455810 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox30: --- → unaffected
status-firefox31: --- → unaffected
status-firefox32: --- → affected
status-firefox33: --- → fixed
https://hg.mozilla.org/mozilla-central/rev/e9d78c3d2eb5
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Depends on: 1039062
https://hg.mozilla.org/releases/mozilla-aurora/rev/3c3f3fa6b3cc
status-firefox32: affected → fixed
Depends on: 1039738

Comment 45

3 years ago
Verified as fixed in build 33 beta 2;
Devices:
- Google Nexus 7 (Android 4.4.4);
- Samsung Galaxy R (Android 2.3.4).
status-firefox33: fixed → verified
You need to log in before you can comment on or make changes to this bug.