Closed Bug 1787964 Opened 3 years ago Closed 3 years ago

Badge designs have inconsistent text-placement in Firefox View

Categories

(Firefox :: Firefox View, defect)

defect

Tracking

()

VERIFIED FIXED
106 Branch
Tracking Status
firefox106 --- verified

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(5 files)

STR:

  1. Sign in to Firefox on several devices (so that you'll have a "Tab Pickup" section in Firefox View".
  2. Visit Firefox View (about:firefoxview)
  3. Compare the "Last active" badge (in the Tab Pickup section) to the "Expires January 23" badge in the colorway section.

ACTUAL RESULTS:
The "Last active" text is biased towards the top of its badge.
The "Expires January 23" text looks centered (maybe even slightly below-center)

EXPECTED RESULTS:
Consistent text alignment within the badge. In particular, "Last Active" should probably be better-centered.

The lack-of-centering here seems to be explicitly specified in CSS with a possibly-hacky pixel-valued padding-bottom decl:
https://searchfox.org/mozilla-central/rev/380fc5571b039fd453b45bbb64ed13146fe9b066/browser/components/firefoxview/firefoxview.css#871,879

.last-active-badge {
...
  padding-bottom: 5px;

Whereas the Expires January 23 badge has consistent top/bottom padding which is why it looks centered:
https://searchfox.org/mozilla-central/rev/380fc5571b039fd453b45bbb64ed13146fe9b066/browser/components/firefoxview/firefoxview.css#285,289

#colorways-collection-expiry-date > span {
...
  padding: .3em 1em;

On macOS the "last active" badge looks less off-center than it does on Linux, but it still does look noticeably top-aligned.

If I just go ahead and put balanced top/bottom padding on .last-active-badge, it doesn't look right. I dug a bit and found out why.

It looks like there's an underlying issue here that we papered over (which the unbalanced padding-bottom:5px was attempting to clean up but not-doing-so robustly). Specifically

  • The text in .last-active-badge lives in a <span class="badge-text"> which has font-size: 0.75em.
  • That span is baseline-aligned to a "full-height" line (~1em tall, not 0.75em tall), in its block container (.last-active-badge)
  • So normally, that text would look like it's biased towards the bottom, since it's baseline-aligned with a taller line in its block container.
  • And it seems someone corrected for that by adding a hardcoded 5px bottom-padding, which makes it look just about right (vertically centered) with some fonts but not others.

We should undo this hack and actually make sure this text fills its line and is properly vertically centered (e.g. with balanced padding on top/bottom).

Suggested fix:
(A) We should move the font-size: 0.75px decl to be on the block (on .last-active-badge) instead of on the span, to avoid getting this weird "smaller text baseline-aligned to the bottom of a larger block" biasing that we were having to correct for.
(B) Then, we can give .last-active-badge "balanced padding" on the top and bottom (whether 0.3em to match the colorway padding, or whatever we like) and the text will look centered.

Also: while we're at it, I noticed that .last-active-tab has a specified width: 6em which is extremely fragile:
https://searchfox.org/mozilla-central/rev/380fc5571b039fd453b45bbb64ed13146fe9b066/browser/components/firefoxview/firefoxview.css#873
We should remove that while we're here. If we don't remove that width, then my suggestion (A) causes "Last active" to linewrap and overflow out of the badge, because obviously 6em is not wide enough to hold the ~11 characters in the text "Last active" plus the dot. (It's barely wide enough in current Nightly due to the 0.75em sizing on the font and the fact that some characters like 't' and 'i' are skinny, but that's extremely fragile and dependent on the system font that we end up using.)

Here are the changes that I made in devtools to produce the improved rendering in comment 2 (with proper/better centering):
(commented out lines are things that I removed; uncommented lines are added)

/* firefoxview.css | chrome://browser/content/firefoxview.css */

.last-active-badge {
  /* width: 6em; */
  /* padding-bottom: 5px; */
  padding: 0.3em 1em;
  font-size: 0.75em;
}

.badge-text {
  /* font-size: .75em; */
}
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Blocks: 1787183

Here's a screenshot of this UI on macOS (where this issue was less-bad on Linux), before the change.

vs. here's a screenshot after the change.

As illustrated by these macOS screenshots, this is a very subtle sizing/positioning change on that platform (still a change that's for-the-better there, IMO).

Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/59be6f72f228 Improve content-based-sizing and vertical centering of Firefox View's "Last Active" badge. r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 106 Branch

Reproduced the initial issue described in comment 0 using old Nightly from 2022-08-29. I verified that using latest Nightly build 107.0a1 and Firefox 106.0b8 across platforms (Windows 10, macOS 11 and Ubuntu 22.04) the text placement inside the Last Active badge is now centred.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: