1387969 - Onboarding layout can break on longer descriptions

Status: VERIFIED FIXED

(Firefox :: General, defect, P3)

Description

[Francesco Lodolo [:flod]]

Attachment: overflow.png

While testing the single search panel, I've noticed how brittle the layout is.

My translation is currently sitting at the very edge, but this f translation is longer it overlaps the "completed" checkbox, and it's not displayed entirely.

Comment 1

[[e-mail]]

Attachment: add maxheight and show scroller with long description

@Verdi, we met a situation that the translated string are too long to display in the normal tour description field.

Either we can do:

1. set content min-height (300px) and show scroller when text is tool long (easy fix, as screenshot)
2. change font size based on text length
(need define the length and font size we'd like to resize)


Which one do you prefer?

Comment 2

[[e-mail]]

I mean set content `max-height`, the style changes are

```
max-height: 300px; (can adjust)
overflow: auto
```

Comment 3

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Another possible choice come to me is scroll across the whole content area including title and description.

Comment 4

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Attachment: Scroll across title and content

like this

Comment 5

[[e-mail]]

flod, is there an existing locale already cut the edge? We'd like to decide if we should fix it and uplift to beta

Comment 6

[Francesco Lodolo [:flod]]

(In reply to Fred Lin [:gasolin] from comment #5)
> flod, is there an existing locale already cut the edge? We'd like to decide
> if we should fix it and uplift to beta

That's really hard to tell, we have almost 90 languages, and it's not even monospaced text.

onboarding.tour-singlesearch.description is by far the longest string we have in onboarding at the moment, but it's 57 only?

https://transvision.mozfr.org/string/?entity=browser/extensions/onboarding/onboarding.properties:onboarding.tour-singlesearch.description&repo=central

French is probably right on the edge, maybe over it.

Having said that, I don't think it would hurt to uplift such a change, worst case you don't show the scrollbar.

Comment 7

[Verdi [:verdi] Best to slack me]

(In reply to Fred Lin [:gasolin] from comment #1)

> Which one do you prefer?

Let's go with the max-height just on the description. Don't include the title. So it should look like your screenshot https://bugzilla.mozilla.org/attachment.cgi?id=8894714

Comment 8

[[e-mail]]

Attachment: Bug 1387969 - show scrollbar when onboarding tour description is longer;

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

Comment 9

[[e-mail]]

At the end I choose 270px in case the title shows in 2 lines... With that setting the scrollbar bottom will align with the top of CTA button.

If the title line could be longer than 2 lines and have longer description, we should consider Rex's option.

Comment 10

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8896180 [details]
Bug 1387969 - show scrollbar when onboarding tour description is longer;

https://reviewboard.mozilla.org/r/167452/#review173274

We do have 3 lines of title in Spanish, unfortunately. :-/
I don't think it's possible to vary the description's height against title's height, so we may need to change the plan. Let’s have a safer plan for all languages.

Comment 11

[[e-mail]]

Attachment: scroll all title and description

Here's how the long title + description will looks like, is it ok for you?

Comment 12

[[e-mail]]

Comment on attachment 8896180 [details]
Bug 1387969 - show scrollbar when onboarding tour description is longer;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/167452/diff/1-2/

Comment 13

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8896180 [details]
Bug 1387969 - show scrollbar when onboarding tour description is longer;

https://reviewboard.mozilla.org/r/167452/#review173362

Comment 14

[Verdi [:verdi] Best to slack me]

(In reply to Fred Lin [:gasolin] from comment #11)
> Created attachment 8896888 [details]
> scroll all title and description
> 
> Here's how the long title + description will looks like, is it ok for you?

I thought we weren't going to scroll the headline. I thought we were just going to scroll the body copy like you did here https://bugzilla.mozilla.org/attachment.cgi?id=8894714

Comment 15

[Verdi [:verdi] Best to slack me]

(In reply to KM Lee [:rexboy] from comment #10)
> Comment on attachment 8896180 [details]
> Bug 1387969 - show scrollbar when onboarding tour description is longer;
> 
> https://reviewboard.mozilla.org/r/167452/#review173274
> 
> We do have 3 lines of title in Spanish, unfortunately. :-/
> I don't think it's possible to vary the description's height against title's
> height, so we may need to change the plan. Let’s have a safer plan for all
> languages.

Oh I missed this comment. Given that, attachment 8896888 [details] looks good.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/160d9f91df68
show scrollbar when onboarding tour description is longer;r=rexboy

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/160d9f91df68

Comment 18

[Justin [:JW_SoftvisionQA]]

I have verified this fix on today's nightly.

Comment 19

[Gerry Chang [:gchang]]

Too late for 56. Mass won't fix for 56.

Comment 20

[Justin [:JW_SoftvisionQA]]

I have verified that the layout is not broken on longer descriptions on Win 10 x64, Win 7 x32, Ubuntu 16.04 x32, and Mac 10.12 with Firefox 58.