Should display the number of not yet complete tours on the onBoarding fox icon

I updated the spec to have the overlay tour icon shown on the new tab page to display a welcome message instead of the number of incomplete tour items.

Update according to bug 1377496 comment 18.

Comment on attachment 8885597 [details] Bug 1357017 - Display a welcome message on the overlay tour icon. https://reviewboard.mozilla.org/r/156456/#review161570 ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:11 (Diff revision 1) > onboarding.hidden-checkbox-label-text=Mark all as complete, and hide the tour > #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. %S is brandShortName. > onboarding.notification-icon-tool-tip=New to %S? > +onboarding.overlay-icon-tool-tip=New to %S? Let's get started. please add LOCALIZATION NOTE when there's a variable

Comment on attachment 8885597 [details] Bug 1357017 - Display a welcome message on the overlay tour icon. https://reviewboard.mozilla.org/r/156456/#review161574 ::: browser/extensions/onboarding/content/onboarding.js:550 (Diff revision 1) > return div; > } > > _renderOverlayIcon() { > let img = this._window.document.createElement("div"); > + img.dataset.tooltip = this._bundle.formatStringFromName("onboarding.overlay-icon-tool-tip", [BRAND_SHORT_NAME], 1); I think it should it be under pref("tour-type") === "new", and we may need confirm the new string for update users

Per comment 6, If we really care about the wording for updated tours, we need the string for: - New user tip for overlay icon - Updated user tip for overlay icon - New user tip for notification - Updated user tip for notification Michael, how do you think? should we have different tips for updated users? if yes, can you give us the messages we should use. Thank you!

And by the way, updated tour is implemented for version 57 or later. In case we can't get it done quickly I'll file another bug dedicated for the updated tour case.

(In reply to KM Lee [:rexboy] from comment #7) > Per comment 6, > If we really care about the wording for updated tours, we need the string > for: > > - New user tip for overlay icon > - Updated user tip for overlay icon > - New user tip for notification > - Updated user tip for notification > > Michael, how do you think? should we have different tips for updated users? > if yes, can you give us the messages we should use. > Thank you! Yes, the fox should say something different for updating users. I'm waiting for feedback from Michelle on the exact message for updating users.

Per engineering meeting we decide to leave updated users to bug 1380963 which targets firefox 57. Michael feel free to update strings on either this bug or bug 1380963.

The patch above

The patch above now depends on patch in a11y bug 1377273, because it changed the DOM structure. and now the patch contains only css changes since the l10n string needed has been included in bug 1377273. Mossop and Gasolin may you help review this patch?

Comment on attachment 8885597 [details] Bug 1357017 - Display a welcome message on the overlay tour icon. https://reviewboard.mozilla.org/r/156456/#review162564

Comment on attachment 8885597 [details] Bug 1357017 - Display a welcome message on the overlay tour icon. https://reviewboard.mozilla.org/r/156456/#review162774 ::: browser/extensions/onboarding/content/onboarding.css:52 (Diff revision 2) > + font-size: 12px; > + border-radius: 22px; > + border: 1px solid #fff; > + padding: 5px 8px; > + text-align: center; > + min-width: 100px; since onboarding-notification messages has very similar UI, could you sync the style between these 2 welcome messages?

Comment on attachment 8885597 [details] Bug 1357017 - Display a welcome message on the overlay tour icon. https://reviewboard.mozilla.org/r/156456/#review162784 ::: browser/extensions/onboarding/content/onboarding.css:52 (Diff revision 2) > + font-size: 12px; > + border-radius: 22px; > + border: 1px solid #fff; > + padding: 5px 8px; > + text-align: center; > + min-width: 100px; from the screenshot, it's better to set a line-height for more balanced 2 line text. ``` line-height: 18px; ``` & please double check the spec, if default is a single lin message, we could try to add ``` min-width: 200px; ``` To make sure the default experience looks identical with spec

Comment on attachment 8885597 [details] Bug 1357017 - Display a welcome message on the overlay tour icon. https://reviewboard.mozilla.org/r/156456/#review162784 > from the screenshot, it's better to set a line-height for more balanced 2 line text. > > ``` > line-height: 18px; > ``` > > & please double check the spec, if default is a single lin message, we could try to add > > ``` > min-width: 200px; > ``` > > To make sure the default experience looks identical with spec The new spec shows 2 line test and don't have additional line height, so we can neglect this

Just had a talk and seems bug 1377273 goes low priority. Let's move the l10n strings here and clear dependence with it. I kept the aria-label from bug 1377273 and read it for displaying the message. :flop would you take a look for the l10n string? We just need to put on one entry here.

Comment on attachment 8885597 [details] Bug 1357017 - Display a welcome message on the overlay tour icon. https://reviewboard.mozilla.org/r/156456/#review163376 Two small improvements on the comments, but it looks good. ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:9 (Diff revision 3) > # LOCALIZATION NOTE(onboarding.overlay-title): This string will be used in the overlay title. %S is brandShortName > onboarding.overlay-title=Getting started with %S > onboarding.hidden-checkbox-label-text=Mark all as complete, and hide the tour > #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. %S is brandShortName. "This string will be used to show the tooltip alongside the notification icon in the notification bar. %S is brandShortName." Added "in the notification bar" to differentiate from the following string. ::: browser/extensions/onboarding/locales/en-US/onboarding.properties:11 (Diff revision 3) > onboarding.hidden-checkbox-label-text=Mark all as complete, and hide the tour > #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. %S is brandShortName. > onboarding.notification-icon-tool-tip=New to %S? > +# LOCALIZATION NOTE(onboarding.overlay-icon-tool-tip): %S is brandShortName. I would add the same note here: "This string will be used to show the tooltip alongside the notification icon in the overlay tour. %S is brandShortName." As said in another bug, not a fan of "tool-tip" in the string ID (it should just be "tooltip").

Comment on attachment 8885597 [details] Bug 1357017 - Display a welcome message on the overlay tour icon. https://reviewboard.mozilla.org/r/156456/#review163376 > I would add the same note here: "This string will be used to show the tooltip alongside the notification icon in the overlay tour. %S is brandShortName." > > As said in another bug, not a fan of "tool-tip" in the string ID (it should just be "tooltip"). Okay, I'll update this and later IDs to tooltip; But leave the entry above that has been checked in.

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s e3a486dbc89a -d afe61e21c394: rebasing 407728:e3a486dbc89a "Bug 1357017 - Display a welcome message on the overlay tour icon. r=flod,mossop" (tip) merging browser/extensions/onboarding/content/onboarding.css merging browser/extensions/onboarding/content/onboarding.js merging browser/extensions/onboarding/locales/en-US/onboarding.properties warning: conflicts while merging browser/extensions/onboarding/content/onboarding.css! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Rebased and re-run the try server. Would you try check-in again, thanks a lot!

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/962cf9dd104c Display a welcome message on the overlay tour icon. r=flod,mossop

Screenshots: https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=1b065ffd8a535a0ad4c39a912af18e948e6a42c1&newProject=mozilla-central&newRev=a1ea69a3a0a760036e047f6ff89897aa7ffd4e53&filter=tabs

I can confirm this is fixed.