1744829 - More From Mozilla 96 experiment UI fixes

Status: VERIFIED FIXED

(Firefox :: Messaging System, enhancement, P1)

Description

[Punam Dahiya [:pdahiya]]

More From Mozilla 96 experiment UI fixes

Comment 1

[Ryan Casey]

Can we change the following:
• "simple" and "advanced" version h3 class="qr-code-box-title" font-size to .87em
• if time, could you alter the sizing of the image in "advanced" to fill the height of the container more? currently the images have more padding on the bottom. See screenshots of before + after for reference.
thank you

Comment 2

[Ryan Casey]

Attachment: before.png

Comment 3

[Ryan Casey]

Attachment: after.png

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

I suspect this wants to be done as part of bug 1744477, which I've marked as blocking this one.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

A couple more issues which are really part of the same code block surfaced in the https://phabricator.services.mozilla.com/D132435 code review. I think this all wants to be fixed together with the changes that Ryan suggested here, because there are a bunch of interdependencies:

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

Comment 6

[Punam Dahiya [:pdahiya]]

[Tracking Requested - why for this release]:
UI fixes needed for Fx96 experiment. Thanks

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Because this patch is only intended for uplift, the things I talked about fixing in comment 5 don't make sense here. I've added them to bug 1744477 comment 0, and marked comment 5 here as obsolete.

Comment 8

[Meg Viar [:mviar]]

Attachment: simple.png

Here's a look at the simple UI with updated font sizes.

Comment 9

[Meg Viar [:mviar]]

Attachment: advanced.png

And here's the advanced UI with updated font sizes and image padding.

Comment 10

[Meg Viar [:mviar]]

Attachment: Bug 1744829 - More From Mozilla 96 experiment UI fixes

Comment 11

[Ryan Casey]

Thanks Meg! those screenshots of ui updates both look great.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7acd674f3bfd
More From Mozilla 96 experiment UI fixes r=pdahiya,preferences-reviewers,mstriemer

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

This really landed in 97.1, but bugzilla doesn't know about that yet, so putting it in 96.3 so we don't lose track of it.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7acd674f3bfd

Comment 15

[Meg Viar [:mviar]]

Comment on attachment 9254633 [details]
Bug 1744829 - More From Mozilla 96 experiment UI fixes

See approval request in next comment

Comment 16

[Meg Viar [:mviar]]

Comment on attachment 9254633 [details]
Bug 1744829 - More From Mozilla 96 experiment UI fixes

Beta/Release Uplift Approval Request

• User impact if declined: Needed for Fx 96 More From Mozilla beta experiment and smaller font sizes could be difficult to read and make the content appear inconsistent
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Changes to font size and padding only
• String changes made/needed: None

Comment 17

[Dianna Smith [:diannaS]]

Comment on attachment 9254633 [details]
Bug 1744829 - More From Mozilla 96 experiment UI fixes

Approved for 96.0b6

Comment 18

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/45b9e4ac8585

Comment 19

[Razvan Trufan]

I have verified this enhancement with the latest Firefox Nightly (97.0a1 Build ID - 20211215215113) installed on Windows 10 x64, MacOS 11.6.1, Linux Mint 20.2. Now I can confirm that font size is .87em and the UI respects the specifications from comment 3.

Comment 20

[Razvan Trufan]

I have verified this enhancement with the latest Firefox Beta (96.0b6 Build ID - 20211216190150) installed on Windows 10 x64, MacOS 11.6.1, Linux Mint 20.2. Now I can confirm that font size is .87em and the UI respects the specifications from comment 3.