Implement new set of feature-focused firstrun panels

VERIFIED FIXED in Firefox 49

Status

()

VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: liuche, Assigned: liuche)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 49
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox49 verified, relnote-firefox 49+)

Details

(URL)

Attachments

(12 attachments)

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
(Assignee)

Description

3 years ago
This includes updating the images and text of first run slides to showcase Tab Queue, Saving, Reader View, and update the sync resources.
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1259833
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1259835
(Assignee)

Comment 3

3 years ago
Let's get some of these assets for the new slides out!
Flags: needinfo?(alam)
Created attachment 8746823 [details]
v3slides_XXHDPI.zip
Flags: needinfo?(alam) → needinfo?(liuche)
(Assignee)

Updated

3 years ago
Flags: needinfo?(liuche)
(Assignee)

Comment 5

3 years ago
Created attachment 8751088 [details]
MozReview Request: Bug 1268647 - Remove obsolete firstrun. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/51241/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51241/
(Assignee)

Comment 6

3 years ago
Created attachment 8751089 [details]
MozReview Request: Bug 1268647 - Add Switch to layout. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/51811/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51811/
(Assignee)

Comment 7

3 years ago
Created attachment 8751090 [details]
MozReview Request: Bug 1268647 - Add localized strings. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/51813/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51813/
(Assignee)

Comment 8

3 years ago
Created attachment 8751091 [details]
MozReview Request: Bug 1268647 - Add to strings.xml.in. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/51815/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51815/
(Assignee)

Comment 9

3 years ago
Created attachment 8751092 [details]
MozReview Request: Bug 1268647 - Add crushed images. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/51817/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51817/
(Assignee)

Comment 10

3 years ago
Created attachment 8751093 [details]
MozReview Request: Bug 1268647 - Add new basic slides. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/51819/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51819/
(Assignee)

Comment 11

3 years ago
Created attachment 8751094 [details]
MozReview Request: Bug 1268647 - Add new sync slide. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/51821/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51821/
(Assignee)

Comment 12

3 years ago
Created attachment 8751095 [details]
MozReview Request: Bug 1268647 - Add Tab queue panel. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/51823/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51823/
(Assignee)

Comment 13

3 years ago
Created attachment 8751096 [details]
MozReview Request: Bug 1268647 - Add control. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/51825/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51825/
(Assignee)

Comment 14

3 years ago
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).

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

3 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.
(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

3 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

3 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

3 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

3 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

3 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

3 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

3 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

3 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

3 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

3 years ago
Attachment #8751103 - Flags: feedback?(alam)
(Assignee)

Comment 26

3 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 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+
Attachment #8751089 - Flags: review?(s.kaspari) → review+
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 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+
Attachment #8751091 - Flags: review?(s.kaspari) → review+
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 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+
Attachment #8751093 - Flags: review?(s.kaspari) → review+
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 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 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+
Attachment #8751096 - Flags: review?(s.kaspari) → review+
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: → bug 1240709
(Assignee)

Comment 37

3 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

3 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.
(Assignee)

Comment 40

3 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

3 years ago
Fixed bustage, forgot to add file to moz.build.
Created attachment 8753796 [details]
Screenshot from 2016-05-18 14:42:27.png

Verified as fixed using:
Device: Moto X (Android 4.4)
Build: Firefox for Android 49.0a1 (2016-05-18)
 
> > 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)
Flags: needinfo?(liuche)
Attachment #8751103 - Flags: feedback?(bbermes)
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: --- → ?
Relnote 49+ added "Updated First Run to showcase several exciting features"
relnote-firefox: ? → 49+
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
status-firefox49: fixed → verified
You need to log in before you can comment on or make changes to this bug.