1371540 - Should add the Library tour in the onBoarding overlay

Status: VERIFIED FIXED

(Firefox :: New Tab Page, enhancement, P2)

Description

[[e-mail]]

Should add the Library tour in the onBoarding overlay

Comment 2

[Fischer [:Fischer]]

Attachment: Bug 1371540 - Should add the Library tour in the onBoarding overlay

This patch
- adds the logic to pick out tours for new user or update user
- add the onboarding library tour

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

Comment 3

[Fischer [:Fischer]]

(In reply to Fischer [:Fischer] from comment #2)
> Created attachment 8880774 [details]
> Bug 1371540 - Should add the Library tour in the onBoarding overlay
> 
> This patch
> - adds the logic to pick out tours for new user or update user
> - add the onboarding library tour
> 
> Review commit: https://reviewboard.mozilla.org/r/152150/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/152150/
This is a skeleton patch.
After the bug 1367696 is landed and the UX specs for FF57 tour set come out, we could proceed to polish and handle the tests.

Comment 4

[[e-mail]]

The patch should update after bug 1375775 is landed

Comment 5

[[e-mail]]

since we got the final string copy in https://docs.google.com/document/d/1abXULIwaMLnXawkLUjpcVawju8tmH0xlVGCOBhuVztM/edit?ts=596d32eb

Let's update the tour and put it into tourset (with the placeholder image). We will update the image once available

Comment 7

[[e-mail]]

Attachment: Bug 1371540 - Should add the Library tour in the onBoarding overlay;

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

Comment 8

[[e-mail]]

Attachment: tour with Hereplaceholder image

Here's the tour with placeholder image

Comment 9

[[e-mail]]

Attachment: notification with placeholder image

Here's the notification with placeholder image

Since library is the v57 tour (but need to land before 8/2), to test this tour on current overlay, we have to set some configs in new profile:

browser.onboarding.newtour=private,library,addons,customize,search,default
browser.onboarding.mute-duration-on-first-session-ms=0

Comment 10

[[e-mail]]

Comment on attachment 8887342 [details]
Bug 1371540 - Should add the Library tour in the onBoarding overlay;

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

Comment 11

[Francesco Lodolo [:flod]]

Comment on attachment 8887342 [details]
Bug 1371540 - Should add the Library tour in the onBoarding overlay;

https://reviewboard.mozilla.org/r/158168/#review163456

::: browser/extensions/onboarding/locales/en-US/onboarding.properties:91
(Diff revision 2)
>  onboarding.notification.onboarding-tour-sync.message=Still sending yourself links to save or read on your phone? Do it the easy way: get Sync and have the things you save here show up on all of your devices.
> +
> +onboarding.tour-library=Library
> +onboarding.tour-library.title=Keep it together.
> +# LOCALIZATION NOTE (onboarding.tour-library.description): This string will be used in the library tour description. %1$S is brandShortName
> +onboarding.tour-library.description=Check out the new %1$S library in the redesigned toolbar. The library puts the things you’ve seen and saved to %1$S - your browsing history,  bookmarks, Pocket lists, and synced tabs - in one convenient place.

Double space before "bookmarks"

Comment 12

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

Comment on attachment 8887342 [details]
Bug 1371540 - Should add the Library tour in the onBoarding overlay;

https://reviewboard.mozilla.org/r/158168/#review163964

::: browser/extensions/onboarding/content/onboarding.js:25
(Diff revision 2)
>                       .createBundle("chrome://branding/locale/brand.properties")
>                       .GetStringFromName("brandShortName");
>  const PROMPT_COUNT_PREF = "browser.onboarding.notification.prompt-count";
>  
>  /**
> - * Add any number of tours, following the format
> + * Add any number of tours, key is the tour id, value should follow the format below

"the tour id" Looks a little bit unclear with the "unique tour id" down there, but I didn't come up a better idea.. :-/

Comment 13

[Dave Townsend [:mossop]]

Comment on attachment 8887342 [details]
Bug 1371540 - Should add the Library tour in the onBoarding overlay;

https://reviewboard.mozilla.org/r/158168/#review164358

Comment 14

[[e-mail]]

Comment on attachment 8887342 [details]
Bug 1371540 - Should add the Library tour in the onBoarding overlay;

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

Comment 15

[[e-mail]]

thanks!

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6bd32f377fca
Should add the Library tour in the onBoarding overlay;r=flod,mossop,rexboy

Comment 17

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

https://hg.mozilla.org/mozilla-central/rev/6bd32f377fca

Comment 18

[Justin [:JW_SoftvisionQA]]

I have verified this fixed on today's nightly.