1268647 - Implement new set of feature-focused firstrun panels

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chenxia Liu [:liuche]]

This includes updating the images and text of first run slides to showcase Tab Queue, Saving, Reader View, and update the sync resources.

Comment 3

[Chenxia Liu [:liuche]]

Let's get some of these assets for the new slides out!

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: v3slides_XXHDPI.zip

Comment 5

[Chenxia Liu [:liuche]]

Attachment: 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/

Comment 6

[Chenxia Liu [:liuche]]

Attachment: 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/

Comment 7

[Chenxia Liu [:liuche]]

Attachment: 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/

Comment 8

[Chenxia Liu [:liuche]]

Attachment: 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/

Comment 9

[Chenxia Liu [:liuche]]

Attachment: 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/

Comment 10

[Chenxia Liu [:liuche]]

Attachment: 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/

Comment 11

[Chenxia Liu [:liuche]]

Attachment: 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/

Comment 12

[Chenxia Liu [:liuche]]

Attachment: 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/

Comment 13

[Chenxia Liu [:liuche]]

Attachment: 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/

Comment 14

[Chenxia Liu [:liuche]]

Attachment: 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.

Comment 15

[Chenxia Liu [:liuche]]

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

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

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

Comment 17

[Chenxia Liu [:liuche]]

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/

Comment 18

[Chenxia Liu [:liuche]]

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/

Comment 19

[Chenxia Liu [:liuche]]

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/

Comment 20

[Chenxia Liu [:liuche]]

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/

Comment 21

[Chenxia Liu [:liuche]]

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/

Comment 22

[Chenxia Liu [:liuche]]

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/

Comment 23

[Chenxia Liu [:liuche]]

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/

Comment 24

[Chenxia Liu [:liuche]]

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/

Comment 25

[Chenxia Liu [:liuche]]

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/

Comment 26

[Chenxia Liu [:liuche]]

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.

Comment 27

[Sebastian Kaspari (:sebastian; :pocmo)]

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?

Comment 28

[Sebastian Kaspari (:sebastian; :pocmo)]

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

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8751090 [details]
MozReview Request: Bug 1268647 - Add localized strings. r=sebastian

https://reviewboard.mozilla.org/r/51813/#review49417

Comment 30

[Sebastian Kaspari (:sebastian; :pocmo)]

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

[Sebastian Kaspari (:sebastian; :pocmo)]

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)

Comment 32

[Sebastian Kaspari (:sebastian; :pocmo)]

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

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8751094 [details]
MozReview Request: Bug 1268647 - Add new sync slide. r=sebastian

https://reviewboard.mozilla.org/r/51821/#review49425

Comment 34

[Sebastian Kaspari (:sebastian; :pocmo)]

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).

Comment 35

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8751096 [details]
MozReview Request: Bug 1268647 - Add control. r=sebastian

https://reviewboard.mozilla.org/r/51825/#review49429

Comment 36

[Sebastian Kaspari (:sebastian; :pocmo)]

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.

Comment 37

[Chenxia Liu [:liuche]]

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.

Comment 38

[Chenxia Liu [:liuche]]

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

[Pulsebot]

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

Comment 40

[Chenxia Liu [:liuche]]

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).

Comment 41

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/c5cfd82c36e5

Comment 42

[Chenxia Liu [:liuche]]

Fixed bustage, forgot to add file to moz.build.

Comment 43

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 44

[Teodora Vermesan (:TeoVermesan)]

Attachment: 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)

Comment 45

[Barbara Bermes [:barbara] - NI please!]

 
> > 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?

Comment 46

[Barbara Bermes [:barbara] - NI please!]

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)]:

Comment 47

[Marcia Knous [:marcia]]

Relnote 49+ added "Updated First Run to showcase several exciting features"

Comment 48

[Sorina Florean [:sflorean]]

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.