Closed Bug 1381407 Opened 6 years ago Closed 6 years ago

Should add the Fast & Modern tour in the onBoarding overlay

Categories

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

Product:
Firefox
Component:
New Tab Page
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 57
Tracking Flags:
Tracking Status
firefox56 --- fixed

People

(Reporter: rexboy, Assigned: rexboy)

References

Details

(Whiteboard: [photon-onboarding])

Attachments

(3 files)

Bug 1381407 - Add Fast&Mordern tour in onboarding overlay.
59 bytes, text/x-review-board-request
mossop
: review+
flod
: review+
gasolin
: review+
Details
wip screenshot 1
1.08 MB, image/png
Details
wip screenshot 2
404.19 KB, image/png
Details 
We need a Fast & Modern tour, which is visible in both new and updated tour sets.

Refer ongoing spec at bug 1381351.
Flags: qe-verify+
QA Contact: jwilliams
Theis is a WIP. I'll update it after specs are locked down.
Comment on attachment 8886993 [details]
Bug 1381407 - Add Fast&Mordern tour in onboarding overlay.

https://reviewboard.mozilla.org/r/157754/#review163282

::: browser/app/profile/firefox.js:1710
(Diff revision 1)
>  // if our notification is finished and safe to show their snippet.
>  pref("browser.onboarding.notification.finished", false);
>  pref("browser.onboarding.notification.mute-duration-on-first-session-ms", 300000); // 5 mins
>  pref("browser.onboarding.notification.max-life-time-per-tour-ms", 432000000); // 5 days
>  pref("browser.onboarding.notification.max-prompt-count-per-tour", 8);
> -pref("browser.onboarding.newtour", "private,addons,customize,singlesearch,default,sync");
> +pref("browser.onboarding.newtour", "performance,private,addons,customize,singlesearch,default,sync");

please don't add this tourset into newtour now,

Therefore we can have this tour inside our tourset but not expose it to v56 users

::: browser/extensions/onboarding/content/onboarding.js:251
(Diff revision 1)
>        `;
>        return div;
>      },
>    },
> +  "performance": {
> +    id: "onboarding-tour-performance",

remember to create a style for the tour list icon
Assignee: nobody → rexboy
Comment on attachment 8886993 [details]
Bug 1381407 - Add Fast&Mordern tour in onboarding overlay.

https://reviewboard.mozilla.org/r/157754/#review163302

::: browser/extensions/onboarding/content/onboarding.js:268
(Diff revision 1)
> +        <section class="onboarding-tour-description">
> +          <h1 data-l10n-id="onboarding.tour-performance.title"></h1>
> +          <p data-l10n-id="onboarding.tour-performance.description"></p>
> +        </section>
> +        <section class="onboarding-tour-content">
> +          <img src="resource://onboarding/img/figure_search.svg" />

please add `role="presentation"` attribute for accessibility
I'll implement "instantly complete tours" mentioned in bug 1381351 comment 4 in this patch.
This tour is not going to be shown until version 57. To reveal the tour, please change the pref:
browser.onboarding.newtour=private,addons,customize,performance,default
browser.onboarding.mute-duration-on-first-session-ms=0
browser.onboarding.notification.tour-ids-queue=onboarding-tour-performance,onboarding-tour-default-browser,onboarding-tour-sync,onboarding-tour-addons,onboarding-tour-default-browser,onboarding-tour-sync
Comment on attachment 8886993 [details]
Bug 1381407 - Add Fast&Mordern tour in onboarding overlay.

https://reviewboard.mozilla.org/r/157754/#review163460
Attachment #8886993 - Flags: review?(francesco.lodolo) → review+
Priority: P2 → P1
Comment on attachment 8886993 [details]
Bug 1381407 - Add Fast&Mordern tour in onboarding overlay.

https://reviewboard.mozilla.org/r/157754/#review164330

::: browser/extensions/onboarding/content/onboarding.css:373
(Diff revision 2)
> +#onboarding-tour-performance:hover {
> +  background-image: url("img/icons_performance-colored.svg");
> +}
> +
> +#onboarding-notification-bar[data-target-tour-id=onboarding-tour-performance] #onboarding-notification-tour-icon {
> +  /* TODO: Placeholder icon. It should be replaced upon assets are available. */

Please file a followup bug for these placeholders and reference it in the comments.

::: browser/extensions/onboarding/content/onboarding.js
(Diff revision 2)
>  const TOUR_AGENT_JS_URI = "resource://onboarding/onboarding-tour-agent.js";
>  const BRAND_SHORT_NAME = Services.strings
>                       .createBundle("chrome://branding/locale/brand.properties")
>                       .GetStringFromName("brandShortName");
>  const PROMPT_COUNT_PREF = "browser.onboarding.notification.prompt-count";
> -

Please keep this line space
Attachment #8886993 - Flags: review?(dtownsend) → review+
Status: NEW → ASSIGNED
Comment on attachment 8886993 [details]
Bug 1381407 - Add Fast&Mordern tour in onboarding overlay.

https://reviewboard.mozilla.org/r/157754/#review164486

::: browser/extensions/onboarding/bootstrap.js:37
(Diff revision 3)
>    "onboarding-tour-customize",
>    "onboarding-tour-search",
>    "onboarding-tour-singlesearch",
>    "onboarding-tour-default-browser",
>    "onboarding-tour-sync",
> +  "onboarding-tour-performance",

please put in alphabet order

::: browser/extensions/onboarding/content/onboarding.js:270
(Diff revision 3)
> +        <section class="onboarding-tour-description">
> +          <h1 data-l10n-id="onboarding.tour-performance.title"></h1>
> +          <p data-l10n-id="onboarding.tour-performance.description"></p>
> +        </section>
> +        <section class="onboarding-tour-content">
> +          <img role="presentation" src="resource://onboarding/img/figure_sync.svg" />

please add role="presentation" at end for consistency
Attachment #8886993 - Flags: review?(gasolin) → review+
Tagging checkin-needed.
Please land bug 1371531 first to prevent conflict. Thanks!
Depends on: 1371531
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d822df13c840
Add Fast&Mordern tour in onboarding overlay. r=flod,gasolin,mossop
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d822df13c840
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
I have verified this fix per comment 9.
Status: RESOLVED → VERIFIED
Blocks: 1390042
You need to log in before you can comment on or make changes to this bug.