Implement the OnBoarding overlay. And we have a rough idea document here https://mozilla.invisionapp.com/share/PXAICZSHU#

In the engineering plan, we have (at least) two questions need to figure out. 1. Do we build the OnBoarding overlay as an (system) Add-on or build-in feature in Firefox? 2. What do we need to do for the OnBoarding Overlay Funnelcake test in engineering work? For instance, just adding a config flag to help build the Funnelcake build or we need to do more?

For the onBoarding overlay, we have to make it not dependent on about:newtab or activity-stream, however, also have to let about:newtab or activity-stream able to display the onBoarding overlay on page load. After discussion with Tim, thanks Tim's input, here is the current plan to achieve the above goal: 1. Build the onBoarding as one components, locating under toolkit/components/onBoarding 2. In the about:newtab or Activity-stream, they include our onBoarding resources and call our onBoarding.init() at the proper moment 3. Inside the onBoarding, we check if - this is a new user (currently see no profile as new user) - not all the tours are completed then we really init the onBoarding codes to do things like appending the overlay into page, controlling the tour, or else just return A test patch[1] has been done. The test patch will make - Have onBoarding.js included in the about:newtab - Once the about:newtab is loaded, call `onBoarding.init(window)` - onBoarding.js would create an overlay then append and display that overlay into page - Clicking outside the overlay would hide the overlay See 0001-This-is-a-try-of-onBoarding.png[2] for the try screenshot. [1] attachment 8858263 [details] [diff] [review]: 0001-This-is-a-try-of-onBoarding.patch [2] attachment 8858262 [details]: 0001-This-is-a-try-of-onBoarding.png

Update the product prototype: https://mozilla.invisionapp.com/share/2HB9YE939#/screens/228699838

I saw fisher has put the entrypoint patch here and I'd like to discuss further about how to organize those OnBoarding tasks. I made a prototype about how to organize tasks, which separate the tasks definition from the basic onboarding flow. https://jsfiddle.net/gasolin/mLpp1dL5/ The benefits are * Separate concern of basic flow and the task specific scripts * developer do not have to deal with the basic flow if he just want to add a new task. * make it more testable * The verify function is async, can support simple or multiple pref condition check. * Auto detect if we need to show the UI * verify each task's condition instead of write a big check function * its flexible if a new task is introduced or an old task is removed * Support `Set all tasks as complete` toggle by default * hook UI with documentFragmenet for performance The main API is onboard.init(hook, rules, width, height); - hook: pass the placeholder element for onboarding UI - rules: define each task(achievement) content and verify conditions Rules is an array of task definition. Here is how a item in Rules array looks like: ``` { id: "task1", // title, badge, ... task: { title: "task 1" }, // content about this task, // return an element for append to the content page: () => { let page = document.createElement("div"); let txt = document.createTextNode("task 1 content"); page.appendChild(txt); return page; }, // script to deal with this page pageActoin: function() {}, // check if this task is done, return a promise verify: async function() { return true; }, // set this task as done (called by setAllComplete) setComplete: async function() { alert("set task1 as done"); }, }, ```

Structure mentioned in comment 7 is hosted at github (with some changes) https://github.com/gasolin/onboarding.js The demo page http://gasolin.github.io/onboarding.js/

Comment on attachment 8861778 [details] Bug 1354046 - Implement the onBoarding overlay architecture https://reviewboard.mozilla.org/r/133776/#review136722 ::: toolkit/components/onBoarding/content/onBoarding.js:32 (Diff revision 1) > + id: "sync", > + title: "Firefox Sync", > + iconSrc: "chrome://global/content/onBoarding/img/dummy-nav-icon.png ", > + page() { > + let div = win.document.createElement("div"); > + div.innerHTML = ` I prefer create and append page with `document.createElement` than innerHTML. Like https://github.com/gasolin/onboarding.js/blob/gh-pages/customize-firefox.js#L8 The content is relatively simple, we can add specific button listener with uitour there. ::: toolkit/components/onBoarding/content/onBoarding.js:241 (Diff revision 1) > + // Although we add the uitour for about:newtab in the permission, it requires to compile. > + // Quite inconvienet, for testing purpose, simply set a test origin. Will remove in the formal patch. > + Services.prefs.setCharPref("browser.uitour.testingOrigins", "about:newtab"); > + > + if (Services.prefs.getBoolPref("browser.onBoarding.disabled", false) || > + Services.prefs.getBoolPref("browser.onBoarding.alwaysHidden", false) The proper condition might be check these 2 conditions `plus` each tour's complete state. Since we should show the overlay when ther is a new tour added. ::: toolkit/components/onBoarding/content/onBoarding.js:245 (Diff revision 1) > + if (Services.prefs.getBoolPref("browser.onBoarding.disabled", false) || > + Services.prefs.getBoolPref("browser.onBoarding.alwaysHidden", false) > + ) { > + return; > + } > + if (!this._isNewOrReturningUser()) { we may not need to implement this for the first run testing. And we should import this function since the same check could be reused for auto migration ::: toolkit/components/onBoarding/content/onBoarding.js:255 (Diff revision 1) > + // no flash of style changes and no additional reflow. > + await this._loadCSS(); > + this._foxIcon = this._renderFoxIcon(); > + this._overlay = this._renderOverlay(); > + this._loadTours(this._overlay, tours); > + this.gotoTour(tours[0].id); can moved into _loadTours ::: toolkit/components/onBoarding/content/onBoarding.js:256 (Diff revision 1) > + await this._loadCSS(); > + this._foxIcon = this._renderFoxIcon(); > + this._overlay = this._renderOverlay(); > + this._loadTours(this._overlay, tours); > + this.gotoTour(tours[0].id); > + let docFrag = win.document.createDocumentFragment(); how about name `docFrag` as `fragment`? ::: toolkit/components/onBoarding/content/onBoarding.js:259 (Diff revision 1) > + this._loadTours(this._overlay, tours); > + this.gotoTour(tours[0].id); > + let docFrag = win.document.createDocumentFragment(); > + docFrag.appendChild(this._foxIcon); > + docFrag.appendChild(this._overlay); > + win.document.body.appendChild(docFrag); we can put all render related process into renderUI, therefore we can have shorten init() ::: toolkit/components/onBoarding/content/onBoarding.js:287 (Diff revision 1) > + // Detroy on unload. This is to ensure we remove all the stuff we left. > + // No any leak out there. > + win.document.addEventListener("unload", () => this.destroy()); > + > + this._initPrefsObserver(); > + this._loadJSLib(); We have to make sure the lib is ready before doing click event

(In reply to Fred Lin [:gasolin] from comment #11) > Comment on attachment 8861778 [details] > Bug 1354046 - Implement the onBoarding overlay architecture > > https://reviewboard.mozilla.org/r/133776/#review136722 > > ::: toolkit/components/onBoarding/content/onBoarding.js:32 > (Diff revision 1) > > + id: "sync", > > + title: "Firefox Sync", > > + iconSrc: "chrome://global/content/onBoarding/img/dummy-nav-icon.png ", > > + page() { > > + let div = win.document.createElement("div"); > > + div.innerHTML = ` > > I prefer create and append page with `document.createElement` than > innerHTML. Like > https://github.com/gasolin/onboarding.js/blob/gh-pages/customize-firefox. > js#L8 > Maybe use the innerHTML would be more reading friendly. > The content is relatively simple, we can add specific button listener with > uitour there. > > ::: toolkit/components/onBoarding/content/onBoarding.js:241 > (Diff revision 1) > > + // Although we add the uitour for about:newtab in the permission, it requires to compile. > > + // Quite inconvienet, for testing purpose, simply set a test origin. Will remove in the formal patch. > > + Services.prefs.setCharPref("browser.uitour.testingOrigins", "about:newtab"); > > + > > + if (Services.prefs.getBoolPref("browser.onBoarding.disabled", false) || > > + Services.prefs.getBoolPref("browser.onBoarding.alwaysHidden", false) > > The proper condition might be check these 2 conditions `plus` each tour's > complete state. Since we should show the overlay when ther is a new tour > added. > Each tour's completion state doesn't matter. Even all tours completed, the overlay would still be there until user explicitly check the hiding-tips-menu checkobox. > ::: toolkit/components/onBoarding/content/onBoarding.js:245 > (Diff revision 1) > > + if (Services.prefs.getBoolPref("browser.onBoarding.disabled", false) || > > + Services.prefs.getBoolPref("browser.onBoarding.alwaysHidden", false) > > + ) { > > + return; > > + } > > + if (!this._isNewOrReturningUser()) { > > we may not need to implement this for the first run testing. And we should > import this function since the same check could be reused for auto migration > Yes, that is a future work. > ::: toolkit/components/onBoarding/content/onBoarding.js:255 > (Diff revision 1) > > + // no flash of style changes and no additional reflow. > > + await this._loadCSS(); > > + this._foxIcon = this._renderFoxIcon(); > > + this._overlay = this._renderOverlay(); > > + this._loadTours(this._overlay, tours); > > + this.gotoTour(tours[0].id); > > can moved into _loadTours > OK, thanks > ::: toolkit/components/onBoarding/content/onBoarding.js:256 > (Diff revision 1) > > + await this._loadCSS(); > > + this._foxIcon = this._renderFoxIcon(); > > + this._overlay = this._renderOverlay(); > > + this._loadTours(this._overlay, tours); > > + this.gotoTour(tours[0].id); > > + let docFrag = win.document.createDocumentFragment(); > > how about name `docFrag` as `fragment`? > OK, thanks > ::: toolkit/components/onBoarding/content/onBoarding.js:259 > (Diff revision 1) > > + this._loadTours(this._overlay, tours); > > + this.gotoTour(tours[0].id); > > + let docFrag = win.document.createDocumentFragment(); > > + docFrag.appendChild(this._foxIcon); > > + docFrag.appendChild(this._overlay); > > + win.document.body.appendChild(docFrag); > > we can put all render related process into renderUI, therefore we can have > shorten init() > Maybe we could do this in the future once the codes get bigger. Right now, not much UI to handle so one less `renderUI` probably be ok. > ::: toolkit/components/onBoarding/content/onBoarding.js:287 > (Diff revision 1) > > + // Detroy on unload. This is to ensure we remove all the stuff we left. > > + // No any leak out there. > > + win.document.addEventListener("unload", () => this.destroy()); > > + > > + this._initPrefsObserver(); > > + this._loadJSLib(); > > We have to make sure the lib is ready before doing click event OK, thanks

Comment on attachment 8861778 [details] Bug 1354046 - Implement the onBoarding overlay architecture Hi Matt, We are working on the Photon onBoarding tour feature. This tour would present a tour overlay to guide user explore Firefox features. Please see the specs[1]. We are targeting the Jun 1 to uplift this pref-controled feature to Beta for the funnelcake testing and will ship for Firefox 57. Before submitting the review patches, we would like to discus with you about our approach so as to ensure the current direction is right and a smoother reviews later. # The requirement for this onBoarding feature: Because uncertain of the landing page, we have to make it not dependent on about:newtab or activity-stream, however, also have to let about:newtab or activity-stream able to display the onBoarding overlay on page load. # Our approach: 1. Create the onBoarding component under toolkit/components/onBoarding 2. In the about:newtab, include onBoarding.js and call the `onBoarding.init()` at the proper moment. 3. During init, onBoarding.js would check if should be hidden or not. 4. onBoarding.js would dynamically include the css and UITour-lib.js 5. onBoarding.js would dynamically create the related elements and append into the DOM. 6. The architecture overview: - one onBoarding.js in charge of managing tours - one onBoarding.css in charge of styles - one `onBoarding` object would be exposed to `window` for outside's use - for some tour completion condiftion. It needs to interact with other page. For example, the addons tour would be marked as completed after user visited the addons page. This patch implemented the current approach and the key functionalities for your feedbacks. That would be greate you could feedback us on - how is the architecture? - is the file location proper? - if some security or performance concern there like about our injection, including external resources etc? - or you see some issue? p.s You will see some dummy UI images, this is because we haven't received the visual assets. We definitely will polish the UI when sending review requests. [1] Spec: https://mozilla.invisionapp.com/share/4MBDUMS5W#/screens/228511972_Overview [2] UX demo page: https://dl.dropboxusercontent.com/u/105549/tour1.framer/index.html Thank you

Comment on attachment 8861778 [details] Bug 1354046 - Implement the onBoarding overlay architecture https://reviewboard.mozilla.org/r/133776/#review137226 ::: toolkit/components/onBoarding/content/onBoarding.js:55 (Diff revision 3) > + isCompleted() { > + return Services.prefs.getBoolPref("browser.onBoarding.tour.sync.completed"); > + }, > + setCompleted() { > + Services.prefs.setBoolPref("browser.onBoarding.tour.sync.completed", true); > + }, we can define related `prefs` in each tour as well, so we can do prefs.addObserver at once and handle register/unregister handlers(isComplete) automatically with onboarding.js

After clicking on the highlighted items it opens the page on the current tab, but on the specification seems it is opened on another tab.

And I guess the situation above is the same to all buttons that use UITour. (e.g. searching on the searching bar will redirect the current tab to search engine) we may need to check if it's acceptable to our scenario.

Hmmm, after some trying it sometimes does open on a new page but sometimes it doesn't. Not very sure about the reason yet.

Target Milestone tracks when the patch landed on m-c and is set automatically by bugherder.

So after our discussions about the implementation options below, which ones are preferred (and why) by the development team? a) System Add-on ** Local frame or remote frame? ** Probably not a webextension since we need to inject in about:newtab b) Remote/Web hosted frame(s) ** Consider the overlay and notification bar ** Deal with interaction with snippets for the notification bar c) Fully built-in d) Use snippets for the notification bar content with UITour APIs to open the overlay and add the fox icon to the page? e) Something else?

Comment on attachment 8861778 [details] Bug 1354046 - Implement the onBoarding overlay architecture Clearing review for now until we get an answer on comment 21.

Hello Matt: We think using system add-on may be a better plan. Something like changing default browser, and listening to push event of firefox sync login, may need a more powerful permission to do it. There was a discussion about how to put on the frame. If we inject it by listen to onload event of gBrowser and inject it by contentDocument.body.appendChild() to about:newtab, we wonder using a global listener creates global pollution to gBrowser. And the similar case if we inject it into tabpanel (maybe need to listen on visibilitychange? and show/hide the overlay correspondingly?). So how about just put necessary files in add-on directory, and still import them from newTab.xhtml to show the overlay? This way we can update necessary things flexibly while avoiding global hooking.

Address some possible consideration: - Our possible way for preference-off may be letting Newtab to load the script, and determine the preference value inside the script to disable the overlay icon. The point is not to create error upon loading newtab page. If loading time matters we may need to do perf-check inside newtab. The script is not going to be a big one so it may be okay. - For css, we may make each rule under .overlay to prevent pollution outside overlay.

So here's the patch for making it a system add-on based on the patch attachment 8861778 [details]; posted here for reference and further discussion. (With a few testing code for fxaccount login things that can be ignored for now)

We're just going to discuss it before having a specific assignee.

Clearing needinfo since the questions were discussed in a meeting

Hi Verdi, Do we only show New and Returning users the onboarding overlay and the notification bar? Thanks.

(In reply to Fischer [:Fischer] from comment #28) > Hi Verdi, > Do we only show New and Returning users the onboarding overlay and the > notification bar? Thanks. Normally, new users and existing users on a new device will see the overlay and the notifications. When we fix Bug 1357666, we'll turn some returning users into new users and they will also see the overlay and notifications. For special updates like Firefox 57, we'll show the overlay and notifications to updating users but it will have different content - an update tour about what's new or different in Firefox. That is Bug 1360378.

Michael, I'm a bit confused between these UX bugs, Based on UX bug 1360474, I guess its option 1: 1. Should display overlay when there's any uncomplete tours after tours replacement (v56: 6 tours in current spec v57: ? tours in current spec + some new tours for 57) Or do you mean the option 2: 2. Should display different tour sets overlay in different condition (v56: 6 tours in current spec v57: 6 tours in current spec + extra tours for 57)

(In reply to Fred Lin [:gasolin] from comment #30) > Michael, I'm a bit confused between these UX bugs, Hi Fred, I made this flowchart to help explain. Let me know if this makes sense. We can talk about it over video too if you'd like.

Thanks for the flowchart diagram, based on our discussion, the overlay should support 2 sets of tours and showing them by condition: * the new user tour-set * the updating user tour-set Some tour in those tour-set maybe overlapped. And we should carry the complete state if user already completed that tour. ex: new user tour-set { tour1: undone, tour2: undone, tour3: done, } updating user tour-set { tour3: done, tour4: undone, tour5: undone, }

This is a P1 bug without an assignee. P1 are bugs which are being worked on for the current release cycle/iteration/sprint. If the bug is not assigned by Monday, 28 August, the bug's priority will be reset to '--'.