Onboarding layout can break on longer descriptions

VERIFIED FIXED in Firefox 57

Status

()

defect
P3
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: flod, Assigned: gasolin)

Tracking

Trunk
Firefox 57
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox56 wontfix, firefox57 verified, firefox58 verified)

Details

(Whiteboard: [photon-onboarding])

Attachments

(5 attachments)

Reporter

Description

2 years ago
Posted image 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.
Whiteboard: [photon-onboarding][triage]
Assignee

Comment 1

2 years ago
@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?
Flags: needinfo?(mverdi)
Assignee

Comment 2

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

```
max-height: 300px; (can adjust)
overflow: auto
```
Another possible choice come to me is scroll across the whole content area including title and description.
Assignee

Comment 5

2 years ago
flod, is there an existing locale already cut the edge? We'd like to decide if we should fix it and uplift to beta
Flags: needinfo?(francesco.lodolo)
Reporter

Comment 6

2 years ago
(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.
Flags: needinfo?(francesco.lodolo)
(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
Flags: needinfo?(mverdi)
Assignee: nobody → gasolin
Flags: qe-verify+
Priority: -- → P3
QA Contact: jwilliams
Whiteboard: [photon-onboarding][triage] → [photon-onboarding]
Comment hidden (mozreview-request)
Assignee

Comment 9

2 years ago
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 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.
Attachment #8896180 - Flags: review?(rexboy)
Assignee

Comment 11

2 years ago
Here's how the long title + description will looks like, is it ok for you?
Flags: needinfo?(mverdi)
Comment hidden (mozreview-request)
Comment on attachment 8896180 [details]
Bug 1387969 - show scrollbar when onboarding tour description is longer;

https://reviewboard.mozilla.org/r/167452/#review173362
Attachment #8896180 - Flags: review?(rexboy) → review+
(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
Flags: needinfo?(mverdi)
(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

2 years ago
Pushed by flin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/160d9f91df68
show scrollbar when onboarding tour description is longer;r=rexboy
Assignee

Updated

2 years ago
Status: NEW → ASSIGNED

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/160d9f91df68
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I have verified this fix on today's nightly.
Status: RESOLVED → VERIFIED
Too late for 56. Mass won't fix for 56.
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.
You need to log in before you can comment on or make changes to this bug.