Closed Bug 1744477 Opened 4 years ago Closed 4 years ago

Make "More from Mozilla" advanced layout fit inside normal pref pane width

Categories

(Firefox :: Messaging System, defect, P2)

defect

Tracking

()

RESOLVED INVALID

People

(Reporter: dmosedale, Assigned: dmosedale)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

In code review, Gijs wrote:

This is too wide, the pane container is set to 664px, https://searchfox.org/mozilla-central/rev/3c9369510cb883b9f08d5f9641e1233d2142f025/browser/themes/shared/preferences/preferences.inc.css#54 .

There are some cases where it still ends up wider, cf. discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1741634 , but we shouldn't make that worse.

I think this might mean the card with the QR code needs a redesign. The text is going to be very squashed in e.g. German, and the font size is already very small. :-\

After playing around, I think this should be relatively straightforward (see above for a first approximation).

It looks to me like things are not as bad as we thought -- setting intl.l10n.pseudo to accented (extends the strings to ~30% longer) on Mac and setting the width to 664 on those boxes mostly looks pretty reasonable.

I suspect that if we put the product icon on the same line as the product name, and found a slightly different design for the "Send an email to your phone instead" string, we could get to a reasonable place with minimal redesign and a bit more CSS tweaking (see the image at the beginning of this comment)

:mstriemer observed that because of the way the layout is constructed, one can click above the "send email" button and still have the link activate. I've confirmed that switching padding-block-start to margin-block-start and tweaking the numbers can fix this, and we may well want to do this in addition to the thing below. However...

:gijs pointed out in the same review that the way is qr-code-box tries to center its content vertically is overly complex and doesn't work very well. He suggested:

I think you can drop this and the other flex: 2 0 auto below and just use justify-content: center on qr-code-box, combined with a minimum margin-bottom on the title in the .simple case? Right now you're using the flex grow of the top and bottom kids of to quasi-center the qr code itself, it feels like, when you could just center the entire thing? Also, I'm pretty sure you don't really want the title to shrink...

which I think is probably required for the right fix to the link clickability issues mentioned above.

:gijs also observed in part of that same review,that the CSS that sets the size of the qr-code-box pegs it at 320px by 204px. He suggested:

There's a larger width pre-set for the content pane next to the sidebars, perhaps this needs to be half that, maybe using a CSS variable + calc?

That said, I think exactly what we do for all this depends on whether we need to change the font-sizes and possibly some design elements for accessibility, which the needinfo on bug 1744477 should help us figure out.

I would set this to iteration 97.1, but bugzilla doesn't know about that one yet.

Here are the CSS changes that were used to make the above layout. We could probably keep most of them, and we'd need to do some other cleanup too, at least.

/* preferences.css | chrome://browser/skin/preferences/preferences.css */

#moreFromMozillaCategory vbox.advanced {
  /* min-width: 725px; */
  /* max-width: 725px; */
  min-width: 664px;
  max-width: 664px;
}

.advanced .qr-code-box {
  /* max-width: 307px; */
  max-width: 245px;
}

#moreFromMozillaCategory .advanced .qr-code-button {
  /* padding-block-start: 30px; */
  padding-block-start: 20px;
}

#moreFromMozillaCategory .advanced .product-title {
  /* padding-inline-start: 0; */
  /* padding-top: 33px; */
  padding-inline-start: 30px;
  padding-top: 0;
  padding: ;
  padding-bottom: 4px;
  overflow-wrap: initial;
}

#moreFromMozillaCategory .advanced .description {
  padding-block-end: 6px;
}

/* Element | about:preferences#moreFromMozilla */

#advanced-qr-code-send-email {
  text-align: center;
}

/* Element | about:preferences#moreFromMozilla */

#firefox-mobile {
  /* margin-bottom: 8px; */
  margin-bottom: 4px;
}
Summary: Make "More from Mozilla" advanced layout fit inside normal pref pane size → Make "More from Mozilla" advanced layout fit inside normal pref pane width
Assignee: nobody → dmosedale
Priority: -- → P1

There is a general a11y review of the More for Mozilla work that :emcminn is planning to drive in bug 1744467.

However, there's one thing that we suspect may require some design changes and could have a big effect on this bug, and so we're trying to get it figured out sooner rather than later: :mstriemer observed that the fonts being used here are quite small, and this could well be an accessibility issue.

:Jamie, could you or someone on your team have a look at these fonts and give us some sense of how things stand from an a11y standpoint? In some idealized world, the current fonts would be good enough, and we could just do something like the work described in comment 0 of this bug and be good to roll this feature out in the 97 release. Realistically, however...

For what it's worth, this pane (as well as the whole prefs UI) is zoomable using Ctrl-0, Ctrl-+ and Ctrl -. On Windows 10, the text seems to respect both "Make text bigger" and "Make everything bigger" in the OS settings.

Here's how to test:

  • Open about:config
  • Set pref 'browser.preferences.moreFromMozilla' as true
  • Set pref 'browser.preferences.moreFromMozilla.template' with string value as 'simple' or 'advanced' to view respective UI
  • Open about:preferences
  • Select the "More from Mozilla" pane
  • Have a look at the fonts in both the 'simple' and 'advanced' layouts.
Flags: needinfo?(jteh)

Asa, would you mind taking a look at this?

Flags: needinfo?(jteh) → needinfo?(asa)
Blocks: 1744829

The fonts are indeed pretty tiny. I don't know what guidance we have on font size. All I could find was the Scale section of https://design.firefox.com/photon/visuals/typography.html

From an accessibility standpoint, I think we want two things. 1) consistency with the rest of the app (particularly the other pref panels) so that if a user is comfortable reading existing UI, they shouldn't be required to make adjustments for new UI. 2) text that scales properly with Firefox and OS settings.

It sounds like we're good for #2 but not so good for #1.

Looking at the More from Mozilla content, I see a font-size: .87em; rule that seems to be responsible for (all? most of?) the differences in "body" text size between this panel and the other pref panels. Can we drop that rule?

Flags: needinfo?(asa)
Blocks: 1744885
Iteration: --- → 96.3 - Nov 29 - Dec 6
Attachment #9255391 - Attachment is obsolete: true
Priority: P1 → P2

The severity field is not set for this bug.
:tspurway, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(tspurway)
Severity: -- → S4
Flags: needinfo?(tspurway)
Iteration: 96.3 - Nov 29 - Dec 6 → 97.3 - Jan 3 - Jan 10
Iteration: 97.3 - Jan 3 - Jan 9 → 98.1 - Jan 10 - Jan 23
Iteration: 98.1 - Jan 10 - Jan 23 → ---
Blocks: 1744467

We're removing the advanced thing; it's not moving forward, so I'm closing this bug.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: