Closed
Bug 1357021
Opened 7 years ago
Closed 6 years ago
Should be able to make all the tours completed by checking "Mark all as complete" checkbox
Categories
(Firefox :: General, defect, P1)
Firefox
General
Tracking
()
VERIFIED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: Fischer, Assigned: Fischer)
References
Details
(Whiteboard: [photon-onboarding])
Attachments
(3 files)
Should be able to make all the tours completed by checking "Mark all as complete" checkbox
Assignee | ||
Updated•7 years ago
|
Whiteboard: [photon-onboarding]
Updated•7 years ago
|
Target Milestone: --- → Firefox 55
Updated•7 years ago
|
Assignee: nobody → rexboy
Updated•7 years ago
|
Status: NEW → ASSIGNED
Updated•7 years ago
|
Flags: qe-verify+
QA Contact: jwilliams
Updated•7 years ago
|
Priority: -- → P1
Updated•6 years ago
|
Priority: P1 → P2
Target Milestone: Firefox 55 → Firefox 57
Updated•6 years ago
|
Assignee: rexboy → gasolin
Comment 1•6 years ago
|
||
should handle this after we implement the first tour validation mechanism
Assignee: gasolin → nobody
Status: ASSIGNED → NEW
Assignee | ||
Comment 2•6 years ago
|
||
(In reply to Fred Lin [:gasolin] from comment #1) > should handle this after we implement the first tour validation mechanism Hi Fred, Would you mind to let me handle this bug? I have some local patch already. I originally would like to tackle it with Bug 1357020. But because of lacking tours right now, I will send out r? afterwards. Thanks
Flags: needinfo?(gasolin)
Comment 4•6 years ago
|
||
For current requirement, I think we should not setComplete all the tours because UX want us to show uncompleted tours in update user tours. So maybe we can just set this item as invalid or wontfix?
Flags: needinfo?(fliu)
Assignee | ||
Comment 5•6 years ago
|
||
Hi Verdi, For the "Make all as complete, and hide the tour" checkbox, Suppose there are tour A, B, C, D in the version 56. Users manually completes tour A only and check "Make all as complete, and hide the tour" checkbox to hide the tours. Later there are new version 57 and in the version 57 there are tour A, B, F, G. We should re-open the hidden tours and show - Case 1: tour A, B both in completed state - Case 2: tour A in completed state, B in not yet completed state Is Case 1 or Case 2 correct? Thanks
Flags: needinfo?(fliu) → needinfo?(mverdi)
Comment 6•6 years ago
|
||
(In reply to Fischer [:Fischer] from comment #5) > Is Case 1 or Case 2 correct? Case 1 would be correct. Let's use a likely use-case. The new user tour in 56 will have: Sync Add-ons One-click Search Private Browsing Customize Default Browser Let's say they either complete these all or checks the box to mark them as complete. The update tour in 57 will have: Sync (same as 56 version) Library Screenshots Customize (different copy from 56 version) Single Search (different from 56 version) The items in the update tour that are the same as the 56 new user tour should then be marked complete (and not get a notification). In this case it would only be Sync. √ Sync Library Screenshots Customize Search
Flags: needinfo?(mverdi)
Comment 7•6 years ago
|
||
Thanks for clarification, so we still need to set all tour as complete when we checked the box and dismissed the overlay.
Assignee: nobody → fliu
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•6 years ago
|
||
(In reply to Fischer [:Fischer] from comment #8) > Created attachment 8878976 [details] > Bug 1357021 - Handle tour completed state > > This commit > - turn on the completed css style for completed tours > - sets individual tour as completed when action button of that tour is > clicked > - sets all tours as completed when hide-the-tour checkbox is checked > > Review commit: https://reviewboard.mozilla.org/r/150232/diff/#index_header > See other reviews: https://reviewboard.mozilla.org/r/150232/ This is a wip patch
Assignee | ||
Comment 15•6 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 18•6 years ago
|
||
(In reply to Fischer [:Fischer] from comment #16) > Created attachment 8879636 [details] > Bug 1357021 - Part 2: Add browser_onboarding_tours.js test, > > This commit > - renames browser_onboarding_hide_tours.js to browser_onboarding_tours.js > - adds test_click_action_button_to_set_tour_completed and > test_set_right_tour_completed_style_on_overlay test cases > - updates test_hide_onboarding_tours test case to test the changes of prefs > of tours complete state > > Review commit: https://reviewboard.mozilla.org/r/150896/diff/#index_header > See other reviews: https://reviewboard.mozilla.org/r/150896/ Hi Mossop, Please see [1] for the UI screenshot. TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef447991c9e616ac4ce60cc1762f554d8c62d657&selectedJob=108567726 [1] attachment 8879632 [details]: onboarding_complete_UI.png Thanks
Assignee | ||
Updated•6 years ago
|
Priority: P2 → P1
Comment 19•6 years ago
|
||
mozreview-review |
Comment on attachment 8878976 [details] Bug 1357021 - Part 1: Handle tours completed state, https://reviewboard.mozilla.org/r/150232/#review156336 ::: browser/extensions/onboarding/content/onboarding.js:291 (Diff revision 2) > + if (params.length > 0) { > + this.sendMessageToChrome("set-prefs", params); > + } > + } > + > + handleTourCompleteState(tourId) { A better name here would be `markTourCompletionState`
Attachment #8878976 -
Flags: review?(dtownsend) → review+
Comment 20•6 years ago
|
||
mozreview-review |
Comment on attachment 8879636 [details] Bug 1357021 - Part 2: Add the browser_onboarding_tours.js test, https://reviewboard.mozilla.org/r/150896/#review156364
Attachment #8879636 -
Flags: review?(dtownsend) → review+
Assignee | ||
Comment 21•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8878976 [details] Bug 1357021 - Part 1: Handle tours completed state, https://reviewboard.mozilla.org/r/150232/#review156336 > A better name here would be `markTourCompletionState` Updated, thanks
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 24•6 years ago
|
||
(In reply to Fischer [:Fischer] from comment #22) > Comment on attachment 8878976 [details] > Bug 1357021 - Part 1: Handle tours completed state, > > Review request updated; see interdiff: > https://reviewboard.mozilla.org/r/150232/diff/2-3/ TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=97ef72c5411e99539afb1b1cf53e15a602a67ce1
Keywords: checkin-needed
Comment 25•6 years ago
|
||
Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2a78b750352f Part 1: Handle tours completed state, r=mossop https://hg.mozilla.org/integration/autoland/rev/3a2a088b9465 Part 2: Add the browser_onboarding_tours.js test, r=mossop
Keywords: checkin-needed
Comment 26•6 years ago
|
||
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
Assignee | ||
Comment 27•6 years ago
|
||
(In reply to Mozilla Autoland from comment #26) > We're sorry - something has gone wrong while rewriting or rebasing your > commits. The commits being pushed no longer match what was requested. Please > file a bug. Hi Tomcat, I am receiving this https://bugzilla.mozilla.org/show_bug.cgi?id=1357021#c26. First time seeing this. What dose this mean? Need me to do any action? thank you.
Flags: needinfo?(cbook)
Comment 28•6 years ago
|
||
(In reply to Fischer [:Fischer] from comment #27) > (In reply to Mozilla Autoland from comment #26) > > We're sorry - something has gone wrong while rewriting or rebasing your > > commits. The commits being pushed no longer match what was requested. Please > > file a bug. > Hi Tomcat, > I am receiving this > https://bugzilla.mozilla.org/show_bug.cgi?id=1357021#c26. First time seeing > this. What dose this mean? Need me to do any action? > thank you. Honestly i don't know - i filed bug 1377065 to get this error message addressed (to file a bug as requested) and we will see there if you/me need to do any action
Flags: needinfo?(cbook)
Comment 29•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2a78b750352f https://hg.mozilla.org/mozilla-central/rev/3a2a088b9465
Assignee | ||
Updated•6 years ago
|
Target Milestone: Firefox 57 → Firefox 56
Comment 31•6 years ago
|
||
I've managed to reproduce this bug on Nightly 55.0a1 (2017-04-17) (64-bit) from Linux x64! This issue is now verified as fixed on Latest Firefox Nightly 56.0a1 (2017-07-06) (64-bit) Build ID: 20170706130309 User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0 OS: Linux 4.8.0-58-generic
QA Whiteboard: [bugday-20170705]
You need to log in
before you can comment on or make changes to this bug.
Description
•