1399177 - Update available doorhanger's "See what's new" link styling looks weird when brandShorterName == Firefox

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P2)

Description

[Alex Thayer [:alexical] (she/her)]

Attachment: brokentext.PNG

I didn't notice this before, because in English, when brandShorterName is "Nightly", the wrapping works out to put "See what's new" on the same line as the text before it. However, when brandShorterName is "Firefox" (on Windows, with standard DPI), "See what's new" gets its own line, creating this weird formatting.

Comment 1

[Alex Thayer [:alexical] (she/her)]

This is off due to the global margin rules for labels. We could override that, or put the link on its own line to match the styling of "Learn more" links in other doorhangers.

Bram, which option do you prefer? Should "See what's new" display in line or should it look like a "Learn more" link?

Comment 2

[Alex Thayer [:alexical] (she/her)]

Attachment: learnmore.PNG

Comment 3

[Alex Thayer [:alexical] (she/her)]

Attachment: seewhatsnew.PNG

Comment 4

[Bram Pitoyo [:bram]]

I propose the latter option: don’t override the global rule, and give the “See what’s new” link its own line to match other doorhangers.

Comment 5

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1399177 - Pull label out of description in update popups

The margin rules for labels mean that if the See what's new link
starts its own line, the alignment looks off. We could override this,
but matching the styling for "Learn more" links, which get their own
line, seemed the better option.

Review commit: https://reviewboard.mozilla.org/r/181950/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/181950/

Comment 6

[:Gijs (he/him)]

Comment on attachment 8910522 [details]
Bug 1399177 - Pull label out of description in update popups

https://reviewboard.mozilla.org/r/181950/#review187460

rs=me, I assume you've tested this. :-)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f81f645704e4
Pull label out of description in update popups r=Gijs

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/f81f645704e4

Comment 9

[Robert Strong (they/them - no direct email)]

Doug, do you think this is worth uplifting to 57?

Comment 10

[Alex Thayer [:alexical] (she/her)]

Hmm, it's a minor issue, but it's also not a very risky change, so I'd say yes.

Comment 11

[Robert Strong (they/them - no direct email)]

Comment on attachment 8910522 [details]
Bug 1399177 - Pull label out of description in update popups

I think this should land for 57

Approval Request Comment
[Feature/Bug causing the regression]: Bug 893505
[User impact if declined]: The UI will look weird when there is a What's New link.
[Is this code covered by automated tests?]: No, this is a visual change
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: I don't think it is needed for this.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It is a styling change
[String changes made/needed]: None

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8910522 [details]
Bug 1399177 - Pull label out of description in update popups

Small polish, taking it (but to be clear, as owner of the release notes, I am a  bit biased ;)
Should be in 57b3

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bc83af76203e

Comment 14

[Andrei Vaida [:avaida]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #11)
> [Is this code covered by automated tests?]: No, this is a visual change
> [Has the fix been verified in Nightly?]: Yes
> [Needs manual test from QE? If yes, steps to reproduce]: I don't think it is
> needed for this.

Setting qe-verify- based on Robert's assessment on manual testing needs.