1042809 - (firstrun) Lightweight first-run experience

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Richard Newman [:rnewman]]

We support downloadable distros. Downloadable distros can modify suggested site tiles. That, and lightweight themes, cause the UI to change after the user sees it.

We've talked about having a minimal first-run experience -- potentially as simple as a splashscreen, but probably more useful -- to 'hide' that flicker, if it's necessary to satisfy partners.

This should be tracking 33, which is the first release that supports downloadable suggested sites.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Tracking 33 would be nice, but let's wait to see what the string situation looks like.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

I think Chenxia is going to start working on this. It aligns with some of the Getting Started work she is involved with too.

Thinking from a technical viewpoint, I think we want:
* A popup window (not fullscreen) that covers most of the phone screen. We could "gray" the application in the background.
* The window only displays on first-run. It would be great to have some iron-clad way to determine first-run, but barring that we could use the same method we use for Telemetry: Creation of the profiles.ini file.
* The window will display some images and text TBD
* The window has a timeout trigger
* The window has a way for the user to close it (touch or button to dismiss)

Comment 3

[Chenxia Liu [:liuche]]

mfinkle, chatted with Yuan, and had a few questions on this:

What's the amount of time that we think we'd need to download the distro? If it's on the order of seconds, Yuan and I were wondering if a user might have dismissed tho overlay by that point.

Also, is this an overlay that is displayed on just builds that need to download a distro, or all builds?

We've got a basic wireframe that should be easy to implement, though I'm fuzzy on how to check for special-distro builds.

One other thing that could be interesting is animating the tiles to "pop out" when the overlay is dismissed (especially for tablet).

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Chenxia Liu [:liuche] from comment #3)
> mfinkle, chatted with Yuan, and had a few questions on this:
> 
> What's the amount of time that we think we'd need to download the distro? If
> it's on the order of seconds, Yuan and I were wondering if a user might have
> dismissed tho overlay by that point.

Don't worry too much about how long it takes to download the distro. We primarily wantprovide some valuable getting started information. The popup does provide a distraction, but that's secondary. Yes, a user may dismiss the popup before the distro downloads, but testing shows that most of the distro is downloaded by ~2 seconds. There might be longer cases, but let's see what more testing shows before we make any drastic changes.

> Also, is this an overlay that is displayed on just builds that need to
> download a distro, or all builds?

All builds. This is the "phase 1" Getting Started popup for all first-runs.


> We've got a basic wireframe that should be easy to implement, though I'm
> fuzzy on how to check for special-distro builds.

Don't even try.
 
> One other thing that could be interesting is animating the tiles to "pop
> out" when the overlay is dismissed (especially for tablet).

That would be cool, but as a new bug.

Comment 5

[Chenxia Liu [:liuche]]

Attachment: Screenshot: tablet

Overlay for tablet. Note that none of the home screen tiles are loaded - when the overlay is dismissed, the titlebar and the tiles will load.

I could probably put the code to launch the overlay somewhere else, but I guess we don't actually want to load any of the tiles as I understand it, in case we do download a distro which has specific tiles to display.

Comment 6

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Phone

This is the full-screen overlay on phones. Ideally we could put some other stuff into that giant gaping space in the middle. It's probably possible to keep a single layout for both tablet and phone though, by adding layout_weight to everything.

Comment 7

[Chenxia Liu [:liuche]]

Attachment: WIP patch: Minimal first run overlay

This is a very basic patch for making the intro screen/overlay, to implement a very basic sketch that Yuan and I talked about.

I'll need to lift out a bunch of styles, and the strings are even hard-coded.

Comment 8

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: Mini onboarding screen on devices_i01.png

Mini onboarding screen on devices:

This onboarding will be taking full screen on Android phone and maintain the same size on Android tablet. 

No auto time out. On tablets, tapping outside of the dialog can dismiss the onboarding screen. On phones, the users can tap "Start Browsing" or slide from right(Optional, depends on the design of control) to dismiss the screen.

This mini onboarding aims to communicate the mission of Firefox products, and provides options to Sign in to Firefox. Currently I think the new tagline of Firefox: "Committed to you, your privacy and an open Web" fits quite nicely. 

I also have some variations of the design. Will attached them in the next comment. Thx!

Comment 9

[Chenxia Liu [:liuche]]

Hehe, looks like we've both got this bug in mind :)

I can change the text really easily - can you upload the icons/images that I should use to this bug at some point?

I'll make the appropriate changes :)

Comment 10

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: Onboarding variations.png

Onboarding screen ideas:
i01: Spilt controls with graphics
- taking the graphics that have been used on Firefox new product website. They align with the tag line quite well. It'd be also nice to animate the icons and Firefox( maybe a pop and icons spinning out) as soon as the page start loading.
- Use split controls to create a separate hierarchy as the main navigation. Works well when there are multiple screens(for future onboarding work)

i02: Single button for "Sign in to Firefox" and slide left/tap to Start Page
- Sliding the page creates a continuity especially when there are multiple onboarding screens to go through. However, it might have discoverability issue for this mini version. 

i03: Simple split controls with no graphics
- A conservative approach, in case people don't like icons ;)

Thoughts?

Comment 11

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

(In reply to Chenxia Liu [:liuche] from comment #9)
> Hehe, looks like we've both got this bug in mind :)
> 
> I can change the text really easily - can you upload the icons/images that I
> should use to this bug at some point?
> 
> I'll make the appropriate changes :)

Thanks Chenxia :)
I would like show the variations above with UX and Product on a mtg tmrw morning. I will keep you updated on the graphics once I get feedback from them.

Comment 12

[Karen Rudnitski [:kar]]

Adding Arcadio so he can keep track of the tag line wording etc.

Comment 13

[Ian Barlow (:ibarlow)]

This is looking great. I really only have one comment:

I want us to make sure we aren't disrupting a user's flow with this overlay, especially if they are just trying to open a link from an external app. There are a couple of ways we might handle that: 

1. just don't show this overlay when a link is loaded via an open or share intent
2. instead of an overlay, consider making this welcome screen "in content", such that a user can either interact with it, or tap the url bar and start browsing.

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Ian Barlow (:ibarlow) from comment #13)
> I want us to make sure we aren't disrupting a user's flow with this overlay,
> especially if they are just trying to open a link from an external app.

Agreed.

> 1. just don't show this overlay when a link is loaded via an open or share
> intent
> 2. instead of an overlay, consider making this welcome screen "in content",
> such that a user can either interact with it, or tap the url bar and start
> browsing.

I like #1 for now - it's simple and meets our goals for downloadable distributions. I think we could move to #2 after we get a more solid plan for first-run tips and getting started.

Comment 15

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Phone overlay v2

I need to add a border to the buttons, and also a spacer view so the tablet version looks balanced.

Comment 16

[Chenxia Liu [:liuche]]

Yuan, can you take a look at the screenshot I have and give me the colors you used for each part? Text color, button highlight color, etc. Thanks!

Comment 17

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: Icons for onboarding screen.zip

Icons and Firefox logo for onboarding screen, provided in 1x and 2x.

I think we should just use the Firefox logo on Aurora and Nightly channel, mainly because based on branding guidelines, Nightly and Aurora logos require using dark background.

Comment 18

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: Mini onboarding_spec_MDPI.png

MDPI spec for onboarding screen.

I would love to add a little animation to the icons. Maybe let them spin out behind the Firefox logo. Animation should not be a blocker though. I can prototype something quick or let Chenxia improvise :)

Comment 19

[Chenxia Liu [:liuche]]

Hey Yuan, would it be easy for you to scale the icons to mdpi, hdpi, and xhdpi? I think 2x is xhdpi, but I'm not entirely sure. Also, it seems like some of the smaller scaled versions will have decimal pixels, because the dimensions are things like 60x61.

antlam showed me this site for calculating icon resizes.
http://coh.io/adpi/

Comment 20

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: Icons for onboarding.zip

Updated icon set in MDPI, HDPI, and XHDPI. 
And Anthony helped fix the decimal pixel issue ;)

Comment 21

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Guest mode overlay tablet

Hey Yuan, I picked some placeholder colors for when the Sync button disabled for Guest mode (since we don't allow guest mode to use Sync).

The text color is #D1D1D1 and the background color is #F5F5F5. Let me know if you want different colors based on how they look.

Comment 22

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Normal overlay phone

Comment 23

[Chenxia Liu [:liuche]]

Attachment: Part 1: Overlay resources

Comment 24

[Chenxia Liu [:liuche]]

Attachment: Part 2: Entry points

Comment 25

[Chenxia Liu [:liuche]]

Yuan, I've put an apk here if you want to try it out:
http://people.mozilla.org/~liuche/bug-1042809/overlay-v1.apk

Comment 26

[Chenxia Liu [:liuche]]

Attachment: Part 1: Overlay resources

(forgot to hg add)

Comment 27

[Chenxia Liu [:liuche]]

Attachment: Part 1: Overlay resources

I ran the screenshots by Yuan on irc and she okay-ed them, so clearing her needinfo and on to code review!

I couldn't come up with a better way of doing image alignments, other than using nested LinearLayouts to control layout_gravity - ugly! Any other ideas, lucas?

Comment 28

[Chenxia Liu [:liuche]]

Attachment: Part 2: Entry points

Comment 29

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Overlay phone 4.1+

Comment 30

[Karen Rudnitski [:kar]]

Chenxia,

I presume 'fennec' is a placeholder and this will be uplifted with 'Firefox'? 

Is it also possible to know whether the tag line will fit nicely when localized to our longer languages (like German) from a visual point of view? If it seems a little long, I would just shorten the tag line to 'committed to you', but will defer on this option until we know what it will look like localized.

Comment 31

[Lucas Rocha (:lucasr)]

Comment on attachment 8471231 [details] [diff] [review]
Part 1: Overlay resources

Review of attachment 8471231 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good overall, still need improvements in the layout.

::: mobile/android/base/StartPane.java
@@ +15,5 @@
> +
> +        // Format newline.
> +        TextView textView = (TextView) findViewById(R.id.subtext);
> +        String subtext = textView.getText().toString();
> +        textView.setText(subtext.replace(STRING_MATCH, "\n"));

STRING_MATCH sounds a bit vague. Also, is this considered good l10n practice? Double-check with l10n folks.

::: mobile/android/base/resources/layout/onboard_start_pane.xml
@@ +7,5 @@
> +
> +    <!-- Empty spacer view -->
> +    <View android:layout_width="0dp"
> +          android:layout_height="0dp"
> +          android:layout_weight="1"/>

It seems to be you could have a view structure roughtly like this:

LinearLayout
 - RelativeLayout (weight = 1)
   Image with addons/private/etc (gravity center)
   Logo image (gravity center with some margin)
 - Title text
 - Subtitle text (weight = 1)
 - Linear layout
   Button 1
   Button 2

@@ +24,5 @@
> +           <ImageView android:layout_width="wrap_content"
> +                      android:layout_height="wrap_content"
> +                      android:layout_gravity="right"
> +                      android:paddingRight="20dp"
> +                      android:src="@drawable/onboard_start_shield"/>

Unless we're planning to animate these images individually, I suggest having all of them in a single image. This should make the view hierarchy a lot simpler.

@@ +56,5 @@
> +                   android:layout_height="wrap_content"
> +                   android:layout_gravity="center"
> +                   android:layout_weight="1"
> +                   android:padding="15sp"
> +                   android:src="@drawable/onboard_start_firefox"/>

Shouldn't this image be branded according to the release channel (nightly aurora, etc)?

@@ +90,5 @@
> +          android:layout_weight="1"/>
> +
> +    <ImageView android:layout_width="match_parent"
> +               android:layout_height="3px"
> +               android:background="@color/android:white"/>

What is this view about? Separator?

@@ +94,5 @@
> +               android:background="@color/android:white"/>
> +
> +    <LinearLayout android:layout_width="match_parent"
> +                  android:layout_height="wrap_content"
> +                  android:orientation="horizontal">

Just set background of this LinearLayout to white and use margins in the child views to fake a divider here. This way you can get rid of the two 'divider' imageviews

::: mobile/android/base/resources/values/colors.xml
@@ +34,5 @@
> +  <!-- Onboarding start pane -->
> +  <color name="onboard_start">#EAEAEA</color>
> +  <color name="onboard_start_highlight">#D8D8D8</color>
> +  <color name="onboard_start_disabled">#F5F5F5</color>
> +  <color name="text_color_onboard_start">#5F636B</color>

Make sure these color aren't already defined somewhere else.

::: mobile/android/base/resources/values/styles.xml
@@ +751,5 @@
>      </style>
>  
> +    <style name="OnboardStartLayout">
> +        <item name="android:layout_width">match_parent</item>
> +        <item name="android:layout_height">match_parent</item>

I generally prefer to keep layout attributes in the layout files instead of styles because the layout files are hard to follow without these attributes i.e. you're creating a level of indirection here. Only define them in a style if you need to vary them according to orientation, form factor, etc.

Comment 32

[Lucas Rocha (:lucasr)]

(In reply to Chenxia Liu [:liuche] from comment #29)
> Created attachment 8471236 [details]
> Screenshot: Overlay phone 4.1+

Some feedback on/questions about the screenshot:
- Is the button text going to fit in other locales? Unlikely. Maybe stack the buttons vertically?
- How does this layout look in landscape?
- Line height on subtext is a bit too tall.

Comment 33

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

(In reply to Karen Rudnitski [:kar] from comment #30)
> Chenxia,
> 
> I presume 'fennec' is a placeholder and this will be uplifted with
> 'Firefox'? 
> 
> Is it also possible to know whether the tag line will fit nicely when
> localized to our longer languages (like German) from a visual point of view?
> If it seems a little long, I would just shorten the tag line to 'committed
> to you', but will defer on this option until we know what it will look like
> localized.

The Firefox website has the full strings localized: https://www.mozilla.org/es-MX/firefox/desktop/ 
I have switched to various locales to see whether the strings are longer. It looks like most full strings in locales are managed to fit in two lines. So I think we shouldn't be concerned about that.

Comment 34

[Richard Newman [:rnewman]]

We usually use Italian and German as our wow-too-long locales, fwiw.

If you're concerned about layout -- and I agree that we should be -- include a thorough localization note in the strings file about how much space is available and how it will flow.

Comment 35

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: Onboarding assets_pngquant_XXHDPI.zip

Updated assets for onboarding.Change log:
* Added icons in XXHDPI
* Added icons of different channels
* All icons have been pngquanted/compressed

Comment 36

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: Locale variations.jpg

After trying the strings in different locales, German and Italian, as examples. 

Chenxia and I agreed to make a few changes on the text style:
* Reduce the font size of subtext from 18px to 16px, in order to accommodate longer strings
* Make subtexts natural wrap
* Reduce the font size of button text from 18px to 16px. Increase the height of action buttons when localized button texts require two lines, for example the German version.

I think these changes will make sure the onboarding screen will be locale friendly. 

I imagine for the upcoming full onboarding work, we will be reusing the navigation buttons and adding onboarding slides on top of that for users to swipe through.

Comment 37

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Landscape phone w/ scrolling

Landscape mode on a phone with a small-ish screen. For future onboarding screens, we want to show the navigation buttons, so also took that into account with this version.

Comment 38

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Layout with various long-text versions

Screenshot with various locales.

Comment 39

[Chenxia Liu [:liuche]]

Attachment: Part 0: Icons and soft links

Adding new branding resources, as well as soft-linking some of the higher-dpi resources that we have to be used, so we don't have to include a ton of new, huge branding icons. (That's okay, right?)

Comment 40

[Chenxia Liu [:liuche]]

Attachment: Part 1: Overlay resources v2

(In reply to Lucas Rocha (:lucasr) from comment #31)
> Comment on attachment 8471231 [details] [diff] [review]
> Part 1: Overlay resources
> 
> Review of attachment 8471231 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good overall, still need improvements in the layout.
> 
> ::: mobile/android/base/StartPane.java
> @@ +15,5 @@
> > +
> > +        // Format newline.
> > +        TextView textView = (TextView) findViewById(R.id.subtext);
> > +        String subtext = textView.getText().toString();
> > +        textView.setText(subtext.replace(STRING_MATCH, "\n"));
> 
> STRING_MATCH sounds a bit vague. Also, is this considered good l10n
> practice? Double-check with l10n folks.

Removed this, because we decided to natural wrap instead. But yeah, I think you're right that that probably isn't l10n-friendly.

> 
> ::: mobile/android/base/resources/layout/onboard_start_pane.xml
> @@ +7,5 @@
> > +
> > +    <!-- Empty spacer view -->
> > +    <View android:layout_width="0dp"
> > +          android:layout_height="0dp"
> > +          android:layout_weight="1"/>
> 
> It seems to be you could have a view structure roughtly like this:
> 
> LinearLayout
>  - RelativeLayout (weight = 1)
>    Image with addons/private/etc (gravity center)
>    Logo image (gravity center with some margin)
>  - Title text
>  - Subtitle text (weight = 1)
>  - Linear layout
>    Button 1
>    Button 2
> 
> @@ +24,5 @@
> > +           <ImageView android:layout_width="wrap_content"
> > +                      android:layout_height="wrap_content"
> > +                      android:layout_gravity="right"
> > +                      android:paddingRight="20dp"
> > +                      android:src="@drawable/onboard_start_shield"/>
> 
> Unless we're planning to animate these images individually, I suggest having
> all of them in a single image. This should make the view hierarchy a lot
> simpler.

I was talking to Yuan, and it does look like we'd want to animate the icons separately, to the extent that they "fly outwards" from the Firefox icon. I ended up keeping the nested LinearLayouts, and added a comment :/

> 
> @@ +56,5 @@
> > +                   android:layout_height="wrap_content"
> > +                   android:layout_gravity="center"
> > +                   android:layout_weight="1"
> > +                   android:padding="15sp"
> > +                   android:src="@drawable/onboard_start_firefox"/>
> 
> Shouldn't this image be branded according to the release channel (nightly
> aurora, etc)?
> 

Good call, done.

> @@ +94,5 @@
> > +               android:background="@color/android:white"/>
> > +
> > +    <LinearLayout android:layout_width="match_parent"
> > +                  android:layout_height="wrap_content"
> > +                  android:orientation="horizontal">
> 
> Just set background of this LinearLayout to white and use margins in the
> child views to fake a divider here. This way you can get rid of the two
> 'divider' imageviews
> 

Thanks! That is exactly what I needed to.

> ::: mobile/android/base/resources/values/colors.xml
> @@ +34,5 @@
> > +  <!-- Onboarding start pane -->
> > +  <color name="onboard_start">#EAEAEA</color>
> > +  <color name="onboard_start_highlight">#D8D8D8</color>
> > +  <color name="onboard_start_disabled">#F5F5F5</color>
> > +  <color name="text_color_onboard_start">#5F636B</color>
> 
> Make sure these color aren't already defined somewhere else.

Yep, not defined anywhere else.

> 
> ::: mobile/android/base/resources/values/styles.xml
> @@ +751,5 @@
> >      </style>
> >  
> > +    <style name="OnboardStartLayout">
> > +        <item name="android:layout_width">match_parent</item>
> > +        <item name="android:layout_height">match_parent</item>
> 
> I generally prefer to keep layout attributes in the layout files instead of
> styles because the layout files are hard to follow without these attributes
> i.e. you're creating a level of indirection here. Only define them in a
> style if you need to vary them according to orientation, form factor, etc.

The layouts I have in this patch are currently the ones that have different requirements for different device types: tablet, v16. The one exception is the styling for the onboard button, which was long (because of padding, margins, text color, drawable background, etc) and looked too cluttered to copy-paste twice. Let me know if you feel otherwise.

Comment 41

[Chenxia Liu [:liuche]]

Attachment: Part 2: Entry points

Removed the Guest mode profile launch, because we don't actually want to show this for guest mode.

Comment 42

[Chenxia Liu [:liuche]]

Yuan, did you hear back from branding about the light/dark backgrounds for other channels?

Also, if you want to try out a build: http://people.mozilla.org/~liuche/bug-1042809/overlay-v2.apk

In particular, take a look at landscape mode (on non-tablets) and let me know if that seems okay.

Comment 43

[Lucas Rocha (:lucasr)]

Comment on attachment 8474015 [details] [diff] [review]
Part 0: Icons and soft links

Review of attachment 8474015 [details] [diff] [review]:
-----------------------------------------------------------------

Ok.

Comment 44

[Lucas Rocha (:lucasr)]

Comment on attachment 8474018 [details] [diff] [review]
Part 1: Overlay resources v2

Review of attachment 8474018 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good, just wonder if you tried using single RelativeLayout to place the images and logo. Let me know if you need to uplift this urgently or something. I don't think we need to block on this if this patch is urgent.

::: mobile/android/base/resources/layout/onboard_start_pane.xml
@@ +21,5 @@
> +
> +            <!-- Empty spacer view -->
> +            <View android:layout_width="0dp"
> +                  android:layout_height="0dp"
> +                  android:layout_weight="1"/>

Do these spacer views actually behave the way you intend here given that the LinearLayout is now inside a ScrollView?

@@ +77,5 @@
> +                           android:layout_height="wrap_content"
> +                           android:layout_gravity="center"
> +                           android:layout_weight="1"
> +                           android:padding="15sp"
> +                           android:src="@drawable/large_icon"/>

Given that we're sticking with view-per-image approach, have you considered/tried using a single RelativeLayout with images aligned to the parent's horizontal center (layout_centerHorizontal=true) and top aligned with the logo (layout_alignTop="@id/image_logo" and then using margins to place them in their relative positions? This would greatly reduce the number of views in this layout.

Comment 45

[Lucas Rocha (:lucasr)]

Comment on attachment 8474020 [details] [diff] [review]
Part 2: Entry points

Review of attachment 8474020 [details] [diff] [review]:
-----------------------------------------------------------------

Let's use a local broadcast instead and keep the start pane triggering logic within BrowserApp.

::: mobile/android/base/GeckoMessageReceiver.java
@@ +14,5 @@
>          final String action = intent.getAction();
>          if (GeckoApp.ACTION_INIT_PW.equals(action)) {
>              GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("Passwords:Init", null));
> +        } else if (GeckoApp.ACTION_NEW_PROFILE.equals(action)) {
> +            BrowserApp.launchStartPane();

This receiver code should live in BrowserApp I think. This static method call in BrowserApp that depends on sActivityContext to be around looks very fragile. Install a local broadcast received when BrowserApp resumes and launch the start pane when the local broadcast is received. Remove receiver when the activity is paused.

::: mobile/android/base/GeckoProfile.java
@@ +657,5 @@
>              Log.w(LOGTAG, "Couldn't write times.json.", e);
>          }
>  
> +        final Intent intent = new Intent(GeckoApp.ACTION_NEW_PROFILE);
> +        mApplicationContext.sendBroadcast(intent);

You should use local broadcast instead so that this intent is only visible within Fennec's process. See:
http://developer.android.com/reference/android/support/v4/content/LocalBroadcastManager.html

HomeConfigPrefsBackend has some code using it, if you're looking for sample code.

::: mobile/android/base/resources/layout/onboard_start_pane.xml
@@ +122,5 @@
>          <Button style="@style/Widget.Onboard.Start.Button"
>                  android:layout_marginTop="3px"
>                  android:layout_marginRight="3px"
> +                android:text="@string/onboard_start_button_account"
> +                android:onClick="showAccountSetup"/>

I'm not a fan of onClick references in layout files as they easily break if you refactor the activity code later. It also forces you to break encapsulation by adding public methods that are supposed to be public.

Comment 46

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Hi Chenxia,

I talked to desktop UX and mentioned about their home pages in different channels. It's fine using Nightly and Auorora logo on the light background. So we should just change the logo and strings for different channels.

Comment 47

[Chenxia Liu [:liuche]]

Attachment: Patch: Overlay resources

I think lucasr is on PTO, so I'm going to send this xml your way, Brian.

Comment 48

[Lucas Rocha (:lucasr)]

Comment on attachment 8475567 [details] [diff] [review]
Patch: Overlay resources

Review of attachment 8475567 [details] [diff] [review]:
-----------------------------------------------------------------

Great. Please test this patch on a device with a smaller resolution to make sure the paddings/margins don't move images off screen. You might need to move some of these paddings to dimension resources so that you can vary them according to each form factor.

Comment 49

[Chenxia Liu [:liuche]]

Attachment: Part 2: Entry points v2

Comment 50

[Lucas Rocha (:lucasr)]

Comment on attachment 8476062 [details] [diff] [review]
Part 2: Entry points v2

Review of attachment 8476062 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. I'd remove the StartPane changes from this patch and move to the previous one. This patch should be just about implementing how the overlay is triggered.

::: mobile/android/base/BrowserApp.java
@@ +553,5 @@
>              "Telemetry:Gather",
>              "Updater:Launch",
>              "BrowserToolbar:Visibility");
>  
> +        registerOnboardingReceiver(appContext);

You can simply use this here. LocalBroadcastManager always calls getApplicationContext() internally. See:
https://github.com/android/platform_frameworks_support/blob/master/v4/java/android/support/v4/content/LocalBroadcastManager.java#L99

@@ +609,5 @@
>          }
>      }
>  
> +    private void registerOnboardingReceiver(Context context) {
> +        LocalBroadcastManager lbm = LocalBroadcastManager.getInstance(context);

Just use 'this' as context here.

nit: final

@@ +615,5 @@
> +        // Receiver for launching first run start pane on new profile creation.
> +        mOnboardingReceiver = new BroadcastReceiver() {
> +            @Override
> +            public void onReceive(Context context, Intent intent) {
> +                launchStartPane(getContext());

BrowserApp.this

@@ +674,5 @@
>          // Register for Prompt:ShowTop so we can foreground this activity even if it's hidden.
>          EventDispatcher.getInstance().registerGeckoThreadListener((GeckoEventListener)this,
>              "Prompt:ShowTop");
> +
> +        LocalBroadcastManager lbm = LocalBroadcastManager.getInstance(getApplicationContext());

getApplicationContext() -> this

nit: final

::: mobile/android/base/GeckoApp.java
@@ +140,5 @@
>      public static final String ACTION_LAUNCH_SETTINGS      = "org.mozilla.gecko.SETTINGS";
>      public static final String ACTION_LOAD                 = "org.mozilla.gecko.LOAD";
>      public static final String ACTION_INIT_PW              = "org.mozilla.gecko.INIT_PW";
>      public static final String ACTION_WEBAPP_PREFIX        = "org.mozilla.gecko.WEBAPP";
> +    public static final String ACTION_NEW_PROFILE          = "org.mozilla.gecko.NEW_PROFILE";

Does it actually make sense to define this in GeckoApp given that this is a BrowserApp-specific intent? I don't feel strongly about it. Up to you.

::: mobile/android/base/GeckoProfile.java
@@ +657,5 @@
>              // Best-effort.
>              Log.w(LOGTAG, "Couldn't write times.json.", e);
>          }
>  
> +        LocalBroadcastManager lbm = LocalBroadcastManager.getInstance(mApplicationContext);

final

::: mobile/android/base/StartPane.java
@@ +14,5 @@
>      @Override
>      public void onCreate(Bundle bundle) {
>          super.onCreate(bundle);
>          setContentView(R.layout.onboard_start_pane);
> +

Same here. This seems to be overlay material. Squash with the patch that implements the UI.

::: mobile/android/base/resources/layout/onboard_start_pane.xml
@@ +111,5 @@
>                    android:orientation="horizontal"
>                    android:background="@color/android:white">
>  
> +        <Button android:id="@+id/button_account"
> +                style="@style/Widget.Onboard.Start.Button"

Unrelated? Looks like this should be in the patch that implements the overlay.

Comment 51

[Chenxia Liu [:liuche]]

https://hg.mozilla.org/integration/fx-team/rev/2680082e54a7
https://hg.mozilla.org/integration/fx-team/rev/7c57a2b1317b
https://hg.mozilla.org/integration/fx-team/rev/09407c4ebcd4

Comment 52

[Chenxia Liu [:liuche]]

AaronMT + QA:
Please try this on multiple screen size devices, and file bugs if anything looks really bad. It also displays differently on tablets.

STR:

1. This only runs on new profile creation, so you'll probably need to clear the app data to see this onboarding screen.

2. Launch Fennec - Onboarding screen should display. This won't show up in history or recent apps.

Comment 53

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

https://hg.mozilla.org/mozilla-central/rev/2680082e54a7
https://hg.mozilla.org/mozilla-central/rev/7c57a2b1317b
https://hg.mozilla.org/mozilla-central/rev/09407c4ebcd4

Comment 54

[Aaron Train [:aaronmt]]

(In reply to Chenxia Liu [:liuche] from comment #52)
> AaronMT + QA:
> Please try this on multiple screen size devices, and file bugs if anything
> looks really bad. It also displays differently on tablets.
> 
> STR:
> 
> 1. This only runs on new profile creation, so you'll probably need to clear
> the app data to see this onboarding screen.
> 
> 2. Launch Fennec - Onboarding screen should display. This won't show up in
> history or recent apps.

We're on it now.

Comment 55

[Chenxia Liu [:liuche]]

Changing the hierarchy on this bug because firstrun is not a metabug.

Comment 56

[Cristina Madaras, QA [:CristinaM]]

Test cases added in Moztrap:
https://moztrap.mozilla.org/manage/case/14606/ - Sign in to Firefox from onboarding screen
https://moztrap.mozilla.org/manage/case/14605/ - Start browsing from onboarding screen

Comment 57

[Lawrence Mandel [:lmandel] (use needinfo)]

Release noted as "Redesigned first run experience"