Closed Bug 1042809 (firstrun) Opened 7 years ago Closed 7 years ago

Lightweight first-run experience

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox34 verified, relnote-firefox 34+, fennec34+)

VERIFIED FIXED
Firefox 34
Tracking Status
firefox34 --- verified
relnote-firefox --- 34+
fennec 34+ ---

People

(Reporter: rnewman, Assigned: liuche)

References

Details

Attachments

(10 files, 16 obsolete files)

1.31 MB, image/png
Details
989.50 KB, image/png
Details
97.65 KB, image/png
Details
253.88 KB, application/zip
Details
309.92 KB, image/jpeg
Details
47.37 KB, image/png
Details
79.52 KB, image/png
Details
178.67 KB, patch
lucasr
: review+
Details | Diff | Splinter Review
19.64 KB, patch
lucasr
: review+
Details | Diff | Splinter Review
11.29 KB, patch
lucasr
: review+
Details | Diff | Splinter Review
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.
tracking-fennec: ? → 34+
Assignee: nobody → ibarlow
Tracking 33 would be nice, but let's wait to see what the string situation looks like.
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)
Assignee: ibarlow → 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).
Flags: needinfo?(mark.finkle)
(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.
Flags: needinfo?(mark.finkle)
Attached image Screenshot: tablet (obsolete) —
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.
Attached image Screenshot: Phone (obsolete) —
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.
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.
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!
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 :)
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?
(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.
Adding Arcadio so he can keep track of the tag line wording etc.
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.
(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.
Summary: Minimal first-run experience for downloadable distributions → Lightweight first-run experience
Attached image Screenshot: Phone overlay v2 (obsolete) —
I need to add a border to the buttons, and also a spacer view so the tablet version looks balanced.
Attachment #8464387 - Attachment is obsolete: true
Attachment #8464389 - Attachment is obsolete: true
Attachment #8464392 - Attachment is obsolete: true
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!
Flags: needinfo?(ywang)
Attached file Icons for onboarding screen.zip (obsolete) —
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.
Flags: needinfo?(ywang)
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 :)
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/
Flags: needinfo?(ywang)
Attached file Icons for onboarding.zip (obsolete) —
Updated icon set in MDPI, HDPI, and XHDPI. 
And Anthony helped fix the decimal pixel issue ;)
Attachment #8467292 - Attachment is obsolete: true
Flags: needinfo?(ywang)
Attached image Screenshot: Guest mode overlay tablet (obsolete) —
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.
Attachment #8465959 - Attachment is obsolete: true
Flags: needinfo?(ywang)
Attached image Screenshot: Normal overlay phone (obsolete) —
Attached patch Part 1: Overlay resources (obsolete) — Splinter Review
Attached patch Part 2: Entry points (obsolete) — Splinter Review
Yuan, I've put an apk here if you want to try it out:
http://people.mozilla.org/~liuche/bug-1042809/overlay-v1.apk
Attached patch Part 1: Overlay resources (obsolete) — Splinter Review
(forgot to hg add)
Attachment #8471168 - Attachment is obsolete: true
Attached patch Part 1: Overlay resources (obsolete) — Splinter Review
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?
Attachment #8471195 - Attachment is obsolete: true
Attachment #8471231 - Flags: review?(lucasr.at.mozilla)
Flags: needinfo?(ywang)
Attached patch Part 2: Entry points (obsolete) — Splinter Review
Attachment #8471169 - Attachment is obsolete: true
Attachment #8471233 - Flags: review?(lucasr.at.mozilla)
Attached image Screenshot: Overlay phone 4.1+ (obsolete) —
Attachment #8471163 - Attachment is obsolete: true
Attachment #8471164 - Attachment is obsolete: true
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 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.
Attachment #8471231 - Flags: review?(lucasr.at.mozilla) → feedback+
(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.
(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.
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.
Attachment #8471233 - Flags: review?(lucasr.at.mozilla) → feedback+
Updated assets for onboarding.Change log:
* Added icons in XXHDPI
* Added icons of different channels
* All icons have been pngquanted/compressed
Attachment #8468079 - Attachment is obsolete: true
Attached image 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.
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.
Attachment #8471236 - Attachment is obsolete: true
Screenshot with various locales.
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?)
Attachment #8471231 - Attachment is obsolete: true
Attachment #8471233 - Attachment is obsolete: true
Attachment #8474015 - Flags: review?(lucasr.at.mozilla)
Attached patch Part 1: Overlay resources v2 (obsolete) — Splinter Review
(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.
Attachment #8474018 - Flags: review?(lucasr.at.mozilla)
Attached patch Part 2: Entry points (obsolete) — Splinter Review
Removed the Guest mode profile launch, because we don't actually want to show this for guest mode.
Attachment #8474020 - Flags: review?(lucasr.at.mozilla)
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.
Flags: needinfo?(ywang)
Comment on attachment 8474015 [details] [diff] [review]
Part 0: Icons and soft links

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

Ok.
Attachment #8474015 - Flags: review?(lucasr.at.mozilla) → review+
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.
Attachment #8474018 - Flags: review?(lucasr.at.mozilla) → feedback+
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.
Attachment #8474020 - Flags: review?(lucasr.at.mozilla) → review-
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.
Flags: needinfo?(ywang)
I think lucasr is on PTO, so I'm going to send this xml your way, Brian.
Attachment #8474018 - Attachment is obsolete: true
Attachment #8475567 - Flags: review?(bnicholson)
Attachment #8475567 - Flags: feedback?(lucasr.at.mozilla)
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.
Attachment #8475567 - Flags: review?(bnicholson)
Attachment #8475567 - Flags: review+
Attachment #8475567 - Flags: feedback?(lucasr.at.mozilla)
Attachment #8474020 - Attachment is obsolete: true
Attachment #8476062 - Flags: review?(lucasr.at.mozilla)
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.
Attachment #8476062 - Flags: review?(lucasr.at.mozilla) → review+
Flags: in-moztrap?(fennec)
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.
Blocks: 1058337
Alias: firstrun
Depends on: 1059440
(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.
Status: RESOLVED → VERIFIED
Depends on: 1059441
Depends on: 1059792
Depends on: 1059827
Changing the hierarchy on this bug because firstrun is not a metabug.
Blocks: 1063748
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
Flags: in-moztrap?(fennec) → in-moztrap+
Depends on: 1065593
Depends on: 1066222
Depends on: 1064538
Depends on: 1063844
Blocks: onboarding
No longer depends on: 1063844, 1064538
Depends on: 1074568
Depends on: 1074570
No longer blocks: 1059792
Depends on: 1059792
Depends on: 1072948
Depends on: 1075032
Alias: firstrun
Alias: firstrun
Depends on: 1081768
Depends on: 1077858
Release noted as "Redesigned first run experience"
Depends on: 1093619
Depends on: 1099436
QA Contact: ioana.chiorean
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.