Closed Bug 1389541 Opened 7 years ago Closed 7 years ago

Onboarding update speech bubble should not wrap

Categories

(Firefox :: General, defect, P3)

Product:
Firefox
Component:
General
Version:
56 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 57
Tracking Flags:
Tracking Status
firefox57 --- verified
firefox58 --- verified

People

(Reporter: verdi, Assigned: rexboy)

Details

(Whiteboard: [photon-onboarding])

Attachments

(2 files)

"Do" wraps to another line
61.10 KB, image/png
Details
Bug 1389541 - Wrap speech bubble of onboarding icon correctly.
59 bytes, text/x-review-board-request
gasolin
: review+
Pike
: review+
Details 
The speech bubble next to the fox should just be 2 lines:

* Nightly is all new.
* See what you can do!

In the screenshot, the second line wraps so that "do!" is on a third line. It should all be together on the second line.
Just made some tries.
A solution can be making use of white-space: pre-line and add \n in l10n entries wherever we want to have a line break.
It Needs to slightly change on l10n strings to include line break though.
Assignee: nobody → rexboy
Comment on attachment 8896849 [details]
Bug 1389541 - Wrap speech bubble of onboarding icon correctly.

https://reviewboard.mozilla.org/r/168142/#review173302

looks good, thanks!
Attachment #8896849 - Flags: review?(gasolin) → review+
Priority: -- → P3
Whiteboard: [photon-onboarding][triage] → [photon-onboarding]
Comment on attachment 8896849 [details]
Bug 1389541 - Wrap speech bubble of onboarding icon correctly.

https://reviewboard.mozilla.org/r/168142/#review173364

I've tried a few extreme texts, and those didn't look good. i.e., the bubble was overlapping with the search box.

I'm generally worried about localizers getting this wrong, so I suggest that you do the CSS assuming they don't have newlines in there.

The localization notes need updates for sure, to at least try to explain this.

And I'm generally not sure how the text should lay out. My examples had a short first line and a long second, which read a bit weird.

I'd attach screenshots, but screenshots doesn't collaborate, and then I lost all my work, and gave up.

::: browser/extensions/onboarding/content/onboarding.css:67
(Diff revision 1)
>    display: inline-block;
>    offset-inline-start: 39px;
>    border-radius: 22px;
> -  padding: 5px 8px;
> -  width: 110px;
> +  padding: 5px 12px;
> +  min-width: 100px;
> +  max-width: 200px;

I think this is too wide?

::: browser/extensions/onboarding/locales/en-US/onboarding.properties:12
(Diff revision 1)
>  #LOCALIZATION NOTE(onboarding.button.learnMore): this string is used as a button label, displayed near the message, and shared across all the onboarding notifications.
>  onboarding.button.learnMore=Learn More
>  # LOCALIZATION NOTE(onboarding.notification-icon-tool-tip): This string will be used to show the tooltip alongside the notification icon in the notification bar. %S is brandShortName.
>  onboarding.notification-icon-tool-tip=New to %S?
>  # LOCALIZATION NOTE(onboarding.overlay-icon-tooltip): This string will be used to show the tooltip alongside the notification icon in the overlay tour. %S is brandShortName.
> -onboarding.overlay-icon-tooltip=New to %S? Let’s get started.
> +onboarding.overlay-icon-tooltip2=New to %S?\nLet’s get started.

This needs an updated localization note, at least. Also, ID needs to match ID.

::: browser/extensions/onboarding/locales/en-US/onboarding.properties:14
(Diff revision 1)
>  # LOCALIZATION NOTE(onboarding.notification-icon-tool-tip): This string will be used to show the tooltip alongside the notification icon in the notification bar. %S is brandShortName.
>  onboarding.notification-icon-tool-tip=New to %S?
>  # LOCALIZATION NOTE(onboarding.overlay-icon-tooltip): This string will be used to show the tooltip alongside the notification icon in the overlay tour. %S is brandShortName.
> -onboarding.overlay-icon-tooltip=New to %S? Let’s get started.
> +onboarding.overlay-icon-tooltip2=New to %S?\nLet’s get started.
>  # LOCALIZATION NOTE(onboarding.overlay-icon-tooltip-updated): %S is brandShortName.
> -onboarding.overlay-icon-tooltip-updated=%S is all new. See what you can do!
> +onboarding.overlay-icon-tooltip-updated2=%S is all new.\nSee what you can do!

Same here.
Attachment #8896849 - Flags: review?(l10n) → review-
One more, do you know what screenreaders do with the newline? Wondering if they'd read that out, and if so, if that's good or bad.
Comment on attachment 8896849 [details]
Bug 1389541 - Wrap speech bubble of onboarding icon correctly.

https://reviewboard.mozilla.org/r/168142/#review173372

::: browser/extensions/onboarding/content/onboarding.css:67
(Diff revision 1)
>    display: inline-block;
>    offset-inline-start: 39px;
>    border-radius: 22px;
> -  padding: 5px 8px;
> -  width: 110px;
> +  padding: 5px 12px;
> +  min-width: 100px;
> +  max-width: 200px;

I'll pick a smaller value. Just thought to give  translators more control on it. But you're right, we should avoid overlapping the search bar.
https://mozilla.invisionapp.com/share/NHCLUXCM7#/screens
and you can see the spec here, from bug 1382376. The line break just follows it.
(In reply to Axel Hecht [:Pike] from comment #5)
> One more, do you know what screenreaders do with the newline? Wondering if
> they'd read that out, and if so, if that's good or bad.

I tested on Voiceover and NVDA, there's no differences between using whitespace and line break.
So I think that should be fine, but let's make a quick ask to a11y team.

Yura, we're trying to add a line break (\n) inside the aria-label="" property of the Overlay button, such that translators can control the line break from l10n entries. Would that cause any trouble for screen readers, from your experiences?
Flags: needinfo?(yzenevich)
I think it's generally fine, unless text is very long, which I believe is not the case here. Screen readers (as you probably saw with Voiceover or NVDA) will read the whole text at once. Especially for actionable elements such as buttons, links etc.
Flags: needinfo?(yzenevich)
Thanks for the answer Yura. I'll prepare the patch later.
Comment on attachment 8896849 [details]
Bug 1389541 - Wrap speech bubble of onboarding icon correctly.

https://reviewboard.mozilla.org/r/168142/#review173950
Attachment #8896849 - Flags: review?(l10n) → review+
try running.
Keywords: checkin-needed
Pushed by kmlee@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5ec2e0d3e78c
Wrap speech bubble of onboarding icon correctly. r=gasolin,Pike
https://hg.mozilla.org/mozilla-central/rev/5ec2e0d3e78c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Flags: qe-verify+
QA Contact: jwilliams
I have verified that this issue is no longer reproducible on Win 10 x64, Win 7 x86, Mac 10.13, & Ubuntu 16.04 x32 with Firefox 58.
status-firefox58: --- → verified
I can confirm this issue is fixed on beta as well. I verified using Fx 57.0b7 on Windows 10 x64, Ubuntu 14.04 LTS and macOS X 10.12.6.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: