Closed
Bug 1268647
Opened 8 years ago
Closed 8 years ago
Implement new set of feature-focused firstrun panels
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox49 verified, relnote-firefox 49+)
VERIFIED
FIXED
Firefox 49
People
(Reporter: liuche, Assigned: liuche)
References
()
Details
Attachments
(12 files)
157.96 KB,
application/zip
|
Details | |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
|
Details |
2.46 MB,
video/mp4
|
Details | |
100.10 KB,
image/png
|
Details |
This includes updating the images and text of first run slides to showcase Tab Queue, Saving, Reader View, and update the sync resources.
Assignee | ||
Comment 3•8 years ago
|
||
Let's get some of these assets for the new slides out!
Flags: needinfo?(alam)
Comment 4•8 years ago
|
||
Flags: needinfo?(alam) → needinfo?(liuche)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(liuche)
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51241/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51241/
Assignee | ||
Comment 6•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51811/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51811/
Assignee | ||
Comment 7•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51813/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51813/
Assignee | ||
Comment 8•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51815/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51815/
Assignee | ||
Comment 9•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51817/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51817/
Assignee | ||
Comment 10•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51819/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51819/
Assignee | ||
Comment 11•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51821/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51821/
Assignee | ||
Comment 12•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51823/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51823/
Assignee | ||
Comment 13•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51825/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51825/
Assignee | ||
Comment 14•8 years ago
|
||
This is running on a Nexus 7 (pre-Marshmallow, so there isn't a prompt requesting the drawOverScreen permission). Some specific feedback questions for you, Barbara and Anthony: I added an auto-advance for 1.2 seconds after tab queue is enabled - this needs a tiny tweak to cancel the auto-advance if the user then toggles this off, but let me know what you think - should we add it in? I have telemetry for turning tab queues on and off. However, I did not add telemetry for the toggle if the permission prompt for drawOverScreen in Marshmallow is shown - do we want telemetry on showing that prompt? And when the user turns on the prompt (or rejects the permission) do we want a separate telemetry probe for that specific case, or is the basic on/off probe enough? Note: For uplifting to Aurora, I'll also have to make switchboard changes to "document" this being en_US, and add code to do this only on en_US.
Assignee: nobody → liuche
Status: NEW → ASSIGNED
Attachment #8751103 -
Flags: feedback?(bbermes)
Attachment #8751103 -
Flags: feedback?(alam)
Assignee | ||
Comment 15•8 years ago
|
||
Also, Barbara, since this is nearly done, let me know about bug 1270641 so that this bug can get uplifted right after we pick a winner through 47.
Comment 16•8 years ago
|
||
(In reply to Chenxia Liu [:liuche] from comment #14) > Created attachment 8751103 [details] > Screencast: Feature first run > > This is running on a Nexus 7 (pre-Marshmallow, so there isn't a prompt > requesting the drawOverScreen permission). Awesome! > Some specific feedback questions for you, Barbara and Anthony: > > I added an auto-advance for 1.2 seconds after tab queue is enabled - this > needs a tiny tweak to cancel the auto-advance if the user then toggles this > off, but let me know what you think - should we add it in? I kinda like this. Especially with the animation, it's obvious that the user can "swipe back". But, how did we arrive at 1.2 seconds? I think 1 second is fine :) That said, are we still allowing the user to press the images to flip the switch? > I have telemetry for turning tab queues on and off. However, I did not add > telemetry for the toggle if the permission prompt for drawOverScreen in > Marshmallow is shown - do we want telemetry on showing that prompt? And when > the user turns on the prompt (or rejects the permission) do we want a > separate telemetry probe for that specific case, or is the basic on/off > probe enough? I'll leave this one for Barbara, but my thoughts are we want to measure the cases where a permission prompt basically creates an obstacle so big for our users that they do not want to enable this feature.
Flags: needinfo?(liuche)
Flags: needinfo?(bbermes)
Assignee | ||
Comment 17•8 years ago
|
||
Comment on attachment 8751088 [details] MozReview Request: Bug 1268647 - Remove obsolete firstrun. r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51241/diff/1-2/
Attachment #8751088 -
Attachment description: MozReview Request: Bug 1268647 - Remove obsolete firstrun. → MozReview Request: Bug 1268647 - Remove obsolete firstrun. r=sebastian
Attachment #8751089 -
Attachment description: MozReview Request: Bug 1268647 - Add Switch. → MozReview Request: Bug 1268647 - Add Switch to layout. r=sebastian
Attachment #8751090 -
Attachment description: MozReview Request: Bug 1268647 - Add localized strings. → MozReview Request: Bug 1268647 - Add localized strings. r=sebastian
Attachment #8751091 -
Attachment description: MozReview Request: Bug 1268647 - Add to strings.xml.in. → MozReview Request: Bug 1268647 - Add to strings.xml.in. r=sebastian
Attachment #8751092 -
Attachment description: MozReview Request: Bug 1268647 - Add crushed images. → MozReview Request: Bug 1268647 - Add crushed images. r=sebastian
Attachment #8751093 -
Attachment description: MozReview Request: Bug 1268647 - Add new basic slides. → MozReview Request: Bug 1268647 - Add new basic slides. r=sebastian
Attachment #8751094 -
Attachment description: MozReview Request: Bug 1268647 - Add new sync slide. → MozReview Request: Bug 1268647 - Add new sync slide. r=sebastian
Attachment #8751095 -
Attachment description: MozReview Request: Bug 1268647 - Add Tab queue panel. → MozReview Request: Bug 1268647 - Add Tab queue panel. r=sebastian
Attachment #8751096 -
Attachment description: MozReview Request: Bug 1268647 - Add auto-advance. → MozReview Request: Bug 1268647 - Add control. r=sebastian
Attachment #8751088 -
Flags: review?(s.kaspari)
Attachment #8751089 -
Flags: review?(s.kaspari)
Attachment #8751090 -
Flags: review?(s.kaspari)
Attachment #8751091 -
Flags: review?(s.kaspari)
Attachment #8751092 -
Flags: review?(s.kaspari)
Attachment #8751093 -
Flags: review?(s.kaspari)
Attachment #8751094 -
Flags: review?(s.kaspari)
Attachment #8751095 -
Flags: review?(s.kaspari)
Attachment #8751096 -
Flags: review?(s.kaspari)
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8751089 [details] MozReview Request: Bug 1268647 - Add Switch to layout. r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51811/diff/1-2/
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8751090 [details] MozReview Request: Bug 1268647 - Add localized strings. r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51813/diff/1-2/
Assignee | ||
Comment 20•8 years ago
|
||
Comment on attachment 8751091 [details] MozReview Request: Bug 1268647 - Add to strings.xml.in. r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51815/diff/1-2/
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8751092 [details] MozReview Request: Bug 1268647 - Add crushed images. r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51817/diff/1-2/
Assignee | ||
Comment 22•8 years ago
|
||
Comment on attachment 8751093 [details] MozReview Request: Bug 1268647 - Add new basic slides. r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51819/diff/1-2/
Assignee | ||
Comment 23•8 years ago
|
||
Comment on attachment 8751094 [details] MozReview Request: Bug 1268647 - Add new sync slide. r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51821/diff/1-2/
Assignee | ||
Comment 24•8 years ago
|
||
Comment on attachment 8751095 [details] MozReview Request: Bug 1268647 - Add Tab queue panel. r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51823/diff/1-2/
Assignee | ||
Comment 25•8 years ago
|
||
Comment on attachment 8751096 [details] MozReview Request: Bug 1268647 - Add control. r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51825/diff/1-2/
Updated•8 years ago
|
Attachment #8751103 -
Flags: feedback?(alam)
Assignee | ||
Comment 26•8 years ago
|
||
I added a probe for when the prompt is shown, and also a probe for when what the user response is to the prompt (accept or reject the permission). I also removed the auto-advance because there were some edge cases that I'd rather not deal with if we want to uplift this to aurora.
Flags: needinfo?(liuche)
Comment 27•8 years ago
|
||
Comment on attachment 8751088 [details] MozReview Request: Bug 1268647 - Remove obsolete firstrun. r=sebastian https://reviewboard.mozilla.org/r/51241/#review49413 ::: mobile/android/base/java/org/mozilla/gecko/util/Experiments.java:40 (Diff revision 2) > - public static final String ONBOARDING2_A = "onboarding2-a"; // Control: Single (blue) welcome screen > - public static final String ONBOARDING2_B = "onboarding2-b"; // 4 static Feature slides > - public static final String ONBOARDING2_C = "onboarding2-c"; // 4 static + 1 clickable (Data saving) Feature slides > + public static final String ONBOARDING3_A = "onboarding3-a"; // Control: No first run > + public static final String ONBOARDING3_B = "onboarding3-b"; // 4 static Feature + 1 dynamic slides > + public static final String ONBOARDING3_C = "onboarding3-c"; // Differentiating features slides Oh, in the "remove" commit is a hidden new configuration of experiments.. :) ::: mobile/android/base/java/org/mozilla/gecko/util/Experiments.java:92 (Diff revision 2) > - if (SwitchBoard.isInBucket(context, 0, 33)) { > - return Experiments.ONBOARDING2_A.equals(experiment); > - } else if (SwitchBoard.isInBucket(context, 33, 66)) { > - return Experiments.ONBOARDING2_B.equals(experiment); > - } else if (SwitchBoard.isInBucket(context, 66, 100)) { > - return Experiments.ONBOARDING2_C.equals(experiment); > + if (SwitchBoard.isInBucket(context, 0, 20)) { > + return Experiments.ONBOARDING3_A.equals(experiment); > + } else if (SwitchBoard.isInBucket(context, 20, 60)) { > + return Experiments.ONBOARDING3_B.equals(experiment); > + } else if (SwitchBoard.isInBucket(context, 60, 100)) { > + return Experiments.ONBOARDING3_C.equals(experiment); If A is no first run and A/B are different first runs. What is bucket 20-40?
Attachment #8751088 -
Flags: review?(s.kaspari) → review+
Updated•8 years ago
|
Attachment #8751089 -
Flags: review?(s.kaspari) → review+
Comment 28•8 years ago
|
||
Comment on attachment 8751089 [details] MozReview Request: Bug 1268647 - Add Switch to layout. r=sebastian https://reviewboard.mozilla.org/r/51811/#review49415 ::: mobile/android/base/resources/layout/firstrun_basepanel_checkable_fragment.xml:43 (Diff revision 2) > android:paddingTop="20dp" > android:paddingBottom="30dp" > android:gravity="center" > android:textAppearance="@style/TextAppearance.FirstrunRegular.Body"/> > > - <View android:layout_weight="1" > + <android.support.v7.widget.SwitchCompat With minSdkVersion 15 we should be able to use 'Switch' from the system framework.
Comment 29•8 years ago
|
||
Comment on attachment 8751090 [details] MozReview Request: Bug 1268647 - Add localized strings. r=sebastian https://reviewboard.mozilla.org/r/51813/#review49417
Attachment #8751090 -
Flags: review?(s.kaspari) → review+
Updated•8 years ago
|
Attachment #8751091 -
Flags: review?(s.kaspari) → review+
Comment 30•8 years ago
|
||
Comment on attachment 8751091 [details] MozReview Request: Bug 1268647 - Add to strings.xml.in. r=sebastian https://reviewboard.mozilla.org/r/51815/#review49419 .. those are small commits .. :)
Comment 31•8 years ago
|
||
Comment on attachment 8751092 [details] MozReview Request: Bug 1268647 - Add crushed images. r=sebastian https://reviewboard.mozilla.org/r/51817/#review49421 (Mozreview really should display images; just found bug 1239879)
Attachment #8751092 -
Flags: review?(s.kaspari) → review+
Updated•8 years ago
|
Attachment #8751093 -
Flags: review?(s.kaspari) → review+
Comment 32•8 years ago
|
||
Comment on attachment 8751093 [details] MozReview Request: Bug 1268647 - Add new basic slides. r=sebastian https://reviewboard.mozilla.org/r/51819/#review49423 ::: mobile/android/base/java/org/mozilla/gecko/firstrun/FirstrunPagerConfig.java:103 (Diff revision 2) > public static final FirstrunPanelConfig urlbarPanelConfig = new FirstrunPanelConfig(FirstrunPanel.class.getName(), R.string.firstrun_panel_title_welcome, R.drawable.firstrun_urlbar, R.string.firstrun_urlbar_message, R.string.firstrun_urlbar_subtext); > public static final FirstrunPanelConfig bookmarksPanelConfig = new FirstrunPanelConfig(FirstrunPanel.class.getName(), R.string.firstrun_bookmarks_title, R.drawable.firstrun_bookmarks, R.string.firstrun_bookmarks_message, R.string.firstrun_bookmarks_subtext); > public static final FirstrunPanelConfig syncPanelConfig = new FirstrunPanelConfig(FirstrunPanel.class.getName(), R.string.firstrun_sync_title, R.drawable.firstrun_sync, R.string.firstrun_sync_message, R.string.firstrun_sync_subtext); > public static final FirstrunPanelConfig dataPanelConfig = new FirstrunPanelConfig(DataPanel.class.getName(), R.string.firstrun_data_title, R.drawable.firstrun_data_off, R.string.firstrun_data_message, R.string.firstrun_data_subtext); > + > + public static final FirstrunPanelConfig notificationsPanelConfig = new FirstrunPanelConfig(FirstrunPanel.class.getName(), R.string.firstrun_notifications_title, R.drawable.firstrun_notifications, R.string.firstrun_notifications_message, R.string.firstrun_notifications_subtext); nit: I wonder why those and SimplePanelConfigs are public/protected. Couldn't this all be private? Is the SimplePanelConfigs class even needed?
Comment 33•8 years ago
|
||
Comment on attachment 8751094 [details] MozReview Request: Bug 1268647 - Add new sync slide. r=sebastian https://reviewboard.mozilla.org/r/51821/#review49425
Attachment #8751094 -
Flags: review?(s.kaspari) → review+
Comment 34•8 years ago
|
||
Comment on attachment 8751095 [details] MozReview Request: Bug 1268647 - Add Tab queue panel. r=sebastian https://reviewboard.mozilla.org/r/51823/#review49427 This diff was a bit difficult to read because of the move DataPanel -> TabQueuePanel. (Maybe it's more of a rm/add than a mv in this case).
Attachment #8751095 -
Flags: review?(s.kaspari) → review+
Updated•8 years ago
|
Attachment #8751096 -
Flags: review?(s.kaspari) → review+
Comment 35•8 years ago
|
||
Comment on attachment 8751096 [details] MozReview Request: Bug 1268647 - Add control. r=sebastian https://reviewboard.mozilla.org/r/51825/#review49429
Comment 36•8 years ago
|
||
FYI: Back when implementing runtime permissions I filed bug 1240709 to improve the prompt for asking for the "draw over other apps" permission. Right now the prompt is just a hacked version of the prompt we show after opening X external links. This works but might not be the best way to get the message across.
See Also: → 1240709
Assignee | ||
Comment 37•8 years ago
|
||
https://reviewboard.mozilla.org/r/51811/#review49473 ::: mobile/android/base/resources/layout/firstrun_basepanel_checkable_fragment.xml:43 (Diff revision 2) > android:paddingTop="20dp" > android:paddingBottom="30dp" > android:gravity="center" > android:textAppearance="@style/TextAppearance.FirstrunRegular.Body"/> > > - <View android:layout_weight="1" > + <android.support.v7.widget.SwitchCompat I originally used Switch, but for some reason, it didn't use the AppCompat styling for Switch for pre-5.0 but then did use AppCompat with 5.0+, so it was inconsistent and didn't show up (and it also wasn't possible to provide track: and thumb: for pre-5.0 and not for 5.0+). Using SwitchCompat fixed these discrepancies between versions though.
Assignee | ||
Comment 38•8 years ago
|
||
https://reviewboard.mozilla.org/r/51819/#review49491 ::: mobile/android/base/java/org/mozilla/gecko/firstrun/FirstrunPagerConfig.java:103 (Diff revision 2) > public static final FirstrunPanelConfig urlbarPanelConfig = new FirstrunPanelConfig(FirstrunPanel.class.getName(), R.string.firstrun_panel_title_welcome, R.drawable.firstrun_urlbar, R.string.firstrun_urlbar_message, R.string.firstrun_urlbar_subtext); > public static final FirstrunPanelConfig bookmarksPanelConfig = new FirstrunPanelConfig(FirstrunPanel.class.getName(), R.string.firstrun_bookmarks_title, R.drawable.firstrun_bookmarks, R.string.firstrun_bookmarks_message, R.string.firstrun_bookmarks_subtext); > public static final FirstrunPanelConfig syncPanelConfig = new FirstrunPanelConfig(FirstrunPanel.class.getName(), R.string.firstrun_sync_title, R.drawable.firstrun_sync, R.string.firstrun_sync_message, R.string.firstrun_sync_subtext); > public static final FirstrunPanelConfig dataPanelConfig = new FirstrunPanelConfig(DataPanel.class.getName(), R.string.firstrun_data_title, R.drawable.firstrun_data_off, R.string.firstrun_data_message, R.string.firstrun_data_subtext); > + > + public static final FirstrunPanelConfig notificationsPanelConfig = new FirstrunPanelConfig(FirstrunPanel.class.getName(), R.string.firstrun_notifications_title, R.drawable.firstrun_notifications, R.string.firstrun_notifications_message, R.string.firstrun_notifications_subtext); That's true, I will make them private. The SimplePanelConfigs class isn't technically needed, but I was using the class to abstract that piece of the code - and it felt messy to have a ton of PanelConfigs floating around, and it was too verbose to instantiate them in the actual code.
Comment 39•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/0d258749a2f6 https://hg.mozilla.org/integration/fx-team/rev/52a530f125b5 https://hg.mozilla.org/integration/fx-team/rev/dc625135601d https://hg.mozilla.org/integration/fx-team/rev/14b8566ac7c0 https://hg.mozilla.org/integration/fx-team/rev/a342654b77bf https://hg.mozilla.org/integration/fx-team/rev/396514782b09 https://hg.mozilla.org/integration/fx-team/rev/e9101f85fe7c https://hg.mozilla.org/integration/fx-team/rev/5187c1c9bd13 https://hg.mozilla.org/integration/fx-team/rev/cf83a5943afc
Assignee | ||
Comment 40•8 years ago
|
||
Landing this with Version C, because there isn't a clear winner. If we decide to go with version B, it'll be easier to remove the dynamic panel (that will turn C into B).
Flags: needinfo?(bbermes)
Assignee | ||
Comment 42•8 years ago
|
||
Fixed bustage, forgot to add file to moz.build.
Comment 43•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0d258749a2f6 https://hg.mozilla.org/mozilla-central/rev/52a530f125b5 https://hg.mozilla.org/mozilla-central/rev/dc625135601d https://hg.mozilla.org/mozilla-central/rev/14b8566ac7c0 https://hg.mozilla.org/mozilla-central/rev/a342654b77bf https://hg.mozilla.org/mozilla-central/rev/396514782b09 https://hg.mozilla.org/mozilla-central/rev/e9101f85fe7c https://hg.mozilla.org/mozilla-central/rev/5187c1c9bd13 https://hg.mozilla.org/mozilla-central/rev/cf83a5943afc https://hg.mozilla.org/mozilla-central/rev/c5cfd82c36e5
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment 44•8 years ago
|
||
Verified as fixed using: Device: Moto X (Android 4.4) Build: Firefox for Android 49.0a1 (2016-05-18)
Comment 45•8 years ago
|
||
> > I have telemetry for turning tab queues on and off. However, I did not add
> > telemetry for the toggle if the permission prompt for drawOverScreen in
> > Marshmallow is shown - do we want telemetry on showing that prompt? And when
> > the user turns on the prompt (or rejects the permission) do we want a
> > separate telemetry probe for that specific case, or is the basic on/off
> > probe enough?
>
> I'll leave this one for Barbara, but my thoughts are we want to measure the
> cases where a permission prompt basically creates an obstacle so big for our
> users that they do not want to enable this feature.
Sorry for the late delay, if possible, we should add telemetry for the prompt as well, and have a simple accepted/not accepted. sounds good?
Flags: needinfo?(liuche)
Updated•8 years ago
|
Flags: needinfo?(liuche)
Updated•8 years ago
|
Attachment #8751103 -
Flags: feedback?(bbermes)
Comment 46•8 years ago
|
||
Release Note Request (optional, but appreciated) [Why is this notable]: [Suggested wording]: Updated First Run with fresh and exciting slides highlighting features [Links (documentation, blog post, etc)]:
relnote-firefox:
--- → ?
Comment 47•8 years ago
|
||
Relnote 49+ added "Updated First Run to showcase several exciting features"
Comment 48•8 years ago
|
||
Verified as fixed in build 49.0a2 (2016-06-15); Device: - Nexus 5 (Android 6.0.1); - Asus ZenPad 8 (Android 5.0.2); - Motorola Razr (Android 4.4.4). And based on comment 44 I will mark this as verified.
Status: RESOLVED → VERIFIED
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•