1021751 - Homepage contextual hint

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ian Barlow (:ibarlow)]

A common piece of user feedback is that people can't find their history or bookmarks. So one of the first "tips" we want to test is something on the homepage that encourages people to swipe left or right to access them. (see https://mobile.etherpad.mozilla.org/feedback-prioritization)

This rough mockup was already in our meta bug 998036 but I noticed we didn't have any bug to track this specific work. http://cl.ly/image/0v0y2G312g1z

Including Anthony here for visual design polish, Chenxia for implementation.

Comment 1

[Aaron Train [:aaronmt]]

(In reply to Ian Barlow (:ibarlow) from comment #0)
> This rough mockup was already in our meta bug 998036 but I noticed we didn't
> have any bug to track this specific work. http://cl.ly/image/0v0y2G312g1z

Have overlays (e.g, http://s3.amazonaws.com/media.nngroup.com/media/editor/2014/02/06/Ness-coachMark.png) that other apps tend to use on first-run been ruled out?

Comment 2

[Ian Barlow (:ibarlow)]

I'm trying to push us away from that. There is a growing body of evidence that suggests people don't read this stuff, and handing out information in smaller, bite sized pieces at the right time is much more valuable.

Comment 3

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

Anthony / Ian: How final are the mockups?

Comment 4

[Ian Barlow (:ibarlow)]

(In reply to Mark Finkle (:mfinkle) from comment #3)
> Anthony / Ian: How final are the mockups?

They can always be more final... I think they are structurally accurate, but depending on what Anthony does in bug 1011712 and bug 1014293 it will likley need a unification pass from him. 

--

A note to the engineer -- consider transitions when building this. It's likely we will want this tip to animate in and animate out in some way.

Comment 5

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

As discussed with Ian, this may be an issue of not associating a "New Tab" with "Bookmarks" or "History". 

The short version of this reasoning is that users are most likely browsing a web page when they encounter a desire and therefore frustration accessing their "bookmarks". At this point, they would just be presented with a URL bar, Tabs button, or 3 dot menu along the top (neither of which are associated with "bookmarks" or "history"). But opening a "new tab" is actually the first thing they must do to access their "bookmarks" and so we can see how there needs to be some sort of mental connection to be made there.

We want to focus more on developing a mental model of "I have shortcuts in my new tab" in the user so they will look to "+" when they want "bookmarks", "history", or maybe even other panels they've then baked into their daily routine.

Mock ups to follow.

Comment 6

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

Attachment: prev_homepage_mock1.png

Adding to my comment above ^...

To reinforce this mental association in the user and show off the side-swipe capability of the Homepage Panels, we're going to try a couple of things:

Mainly, I would like to try and ease in (with a little bit of a bounce and delay) the labels that exist to the right and left of "Top Sites".

I also want to extend the height of the navigational header where the labels currently reside ("Top sites", "Bookmarks", etc) and utilize the snippet to display some additional information.

Comment 7

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

Attachment: onload_exp3.mov

Here's a quick animation clip to show what I mean. Timing here would be key, so we have to detail and fine tune this.

If it helps, it's set at 2.25 seconds delay right now.

Comment 8

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

Looks like we could tie into the banner system to display the mockup. The animation clip isn't really showing any animation though. Might be a .mov issue?

I was talking to Chenxia about this bug and proposed that we could use something like Ian's mockup in bug 1011712 (https://bugzilla.mozilla.org/attachment.cgi?id=8427788) here if we wanted. The original banner (aligned to top) is causing a little grief in implementation given that it sits in "content" and would even be in conflict with promo banners at times.

The popup "PRO TIP" approach gives more of a "tip" feel and can be made to be low-obtrusive, but offering the user a "this thing means something different than other UI in the app" feel. It seems like we are striving for a consistency in tips as well, and I like having a solid base fo code being reused in other situations. It makes adding new tip faster in the future (at least in places where the tip popup is appropriate).

Comment 9

[Chenxia Liu [:liuche]]

In order to animate the titles in the home pager tab strip, we'll need to either 1) overlay a fake tab strip on top of the Android PagerTabStrip and hide it after the animation, or 2) copy the code for PagerTabStrip and PagerTitleStrip and make those elements protected (instead of package-private).

I'm going to experiment with (2) because if we want this animation all the time, taking the approach of (1) and overlaying a FrameLayout will probably make for some negative perf, and we'll also have to handle the case where the user swipes during the animation.

Comment 10

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

Both of these approaches are less than good. I'm not keen on copying Android source code, and dealing with the maintenance, for this feature.

Comment 11

[Ian Barlow (:ibarlow)]

Friendly ping on status here?

Comment 12

[Chenxia Liu [:liuche]]

I have a proof of concept with ripping ~100k of code from the Android source, but I started trying to finish some of the "close all tabs"/recent pages work. I'll finish up my rough mock and post something for UX to try in the next day or two.

Comment 13

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

Were you going to look into using reflection too?

Comment 14

[Chenxia Liu [:liuche]]

Yep, that's what I'm looking at now.

Comment 15

[Chenxia Liu [:liuche]]

This is obviously not going to make 33, so dropping that flag.

Comment 16

[Chenxia Liu [:liuche]]

Attachment: Patch: Titlebar bounce

antlam, here's an apk for you to try:
http://people.mozilla.org/~liuche/bug-1021751/title-bounce1.apk

Let me know about the timing.

Comment 17

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

Hey Chenxia!

Awesome work! Seeing the animation is nice. 

In regards to timing, I'd like to try a .25 second delay if possible. Could we also get it to slide in faster and bounce with a little less tension? 

It's definitely close, I think we just need to fine tune it. :)

Comment 18

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

To bring this back to the discover-ability issue - have we thought about also exposing these two as menu options tucked into the "3 dot" overflow icon?

Comment 19

[Chenxia Liu [:liuche]]

Anthony and I have been going back and forth on IRC, but the various tweaked apks are here:
http://people.mozilla.org/~liuche/bug-1021751/

Comment 20

[Chenxia Liu [:liuche]]

Attachment: Patch: WIP Homescreen contextual hint v2

Currently going back and forth with antlam about timing and the feel of the animation.

This is the current iteration, and has a somewhat complex chain of animations to get the correct bounce.

Comment 21

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

Thanks for all the builds Chenxia! It's coming along nicely!

I'm testing out v.4 right now and I think it overshoots too much on the first time around. It also seems like it doesn't bounce "back" enough (I think it stops right now as soon as it comes back around?)

You mentioned that this was essentially setting multiple "destination" points for the labels to hit, might I ask how many we have right now?

Comment 22

[Lucas Rocha (:lucasr)]

Comment on attachment 8460666 [details] [diff] [review]
Patch: WIP Homescreen contextual hint v2

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

Let's not rely on reflection. Animation looks good, I'd like to see it refactored out into its own class.

::: mobile/android/base/home/HomePagerTabStrip.java
@@ +59,5 @@
> +        try {
> +            // Use reflection to make the Tab Strip titles accessible for animation.
> +            final Field prevText = PagerTitleStrip.class.getDeclaredField("mPrevText");
> +            prevText.setAccessible(true);
> +            final View prevTextView = (View) prevText.get(this);

Can't you simply do a getChildAt(0) here? As you know, this is likely to break in the future.

@@ +63,5 @@
> +            final View prevTextView = (View) prevText.get(this);
> +
> +            final Field nextText = PagerTitleStrip.class.getDeclaredField("mNextText");
> +            nextText.setAccessible(true);
> +            final View nextTextView = (View) nextText.get(this);

Same here, maybe just do getChildAt(getChildCount() - 1)?

Just bail (instead of crashing) if these views cannot be found.

@@ +81,5 @@
> +            // Two-part animator for a softer bounce.
> +            final PropertyAnimator softBounceAnimator = new PropertyAnimator(ANIMATION2_DURATION_MS, new AccelerateInterpolator());
> +            softBounceAnimator.attach(prevTextView, Property.TRANSLATION_X, -bounceDistance + BOUNCE_OFFSET);
> +            softBounceAnimator.attach(nextTextView, Property.TRANSLATION_X, bounceDistance - BOUNCE_OFFSET);
> +            softBounceAnimator.addPropertyAnimationListener(new PropertyAnimationListener() {

Maybe factor out this bounce animation code into a separate class or something?

@@ +92,5 @@
> +                    // Bounce animation that leaves the text in its original position.
> +                    final PropertyAnimator bounceAnimator = new PropertyAnimator(20, new BounceInterpolator());
> +                    bounceAnimator.attach(prevTextView, Property.TRANSLATION_X, 0);
> +                    bounceAnimator.attach(nextTextView, Property.TRANSLATION_X, 0);
> +                    bounceAnimator.start();

This reminds me we should revisit that idea of switching to NineOldAndroids...

@@ +115,5 @@
> +                @Override
> +                public void run() {
> +                    animator.start();
> +                }
> +            }, ANIMATION_DELAY_MS);

Why the delay?

::: mobile/android/base/resources/layout/home_pager.xml
@@ +13,5 @@
>                                    android:layout_height="match_parent"
>                                    android:background="@android:color/white">
>  
>      <org.mozilla.gecko.home.HomePagerTabStrip android:layout_width="match_parent"
> +                                              android:layout_height="40dip"

Maybe this should go in a separate patch?

Comment 23

[Chenxia Liu [:liuche]]

Anthony, a few more apks for you to try:

http://people.mozilla.org/~liuche/bug-1021751/

See 5, 6A, and 6B.

Comment 24

[Chenxia Liu [:liuche]]

Anthony, I added another apk that has a smaller bounce and also does the stairstep bounce *mimes some hand motions*

See 7-small-bounce.apk at http://people.mozilla.org/~liuche/bug-1021751/

I can tweak a lot of things there easily now - duration of each leg of the bounce, distance of each leg, fading speed, etc. Let me know what you think.

Comment 25

[Chenxia Liu [:liuche]]

Attachment: Part 2-ish: BounceAnimator

This is a custom BounceAnimator that chains AccelerateInterpolators so that in the future, we can have more control over making bounce animations the way we want instead of using Android's BounceInterpolator (which is hard-coded to have 4 bounces).

Comment 26

[Chenxia Liu [:liuche]]

Attachment: Part 3-ish: Homepage with BounceAnimator

This will eventually be merged with the Homescreen contextual hint patch, but is just a WIP demonstrating the use of the BounceAnimator.

Comment 27

[Chenxia Liu [:liuche]]

Comment on attachment 8465964 [details] [diff] [review]
Part 2-ish: BounceAnimator

On second thought, I'll ask for feedback if this turns out to be something that antlam likes.

Comment 28

[Chenxia Liu [:liuche]]

Attachment: Part 3-ish: Homepage with BounceAnimator

(oops, uploaded a completely empty patch)

Comment 29

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

Attachment: icon_home.zip

Animation looks great since the 8th iteration. I think next step will just be to wrap up the snippet and then we can wrap this up and let it simmer for a bit. 

I'm really interested to see how all the elements come together to help build more context around about:home in the users mind.

Comment 30

[Chenxia Liu [:liuche]]

Attachment: Part 1: Bounce animator

Bounce animator - need to fix gravity in the titlebar :/

Comment 31

[Chenxia Liu [:liuche]]

Attachment: Part 2: Home page snippet

Comment 32

[Lucas Rocha (:lucasr)]

Comment on attachment 8476396 [details] [diff] [review]
Part 2: Home page snippet

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

Looking pretty good overall. Just not entirely sure about the telemetry aspect and copy. Up to you to decide.

::: mobile/android/components/Snippets.js
@@ +384,5 @@
> +  let id = Home.banner.add({
> +    text: text,
> +    icon: "drawable://homepage_banner",
> +    onclick: function() {
> +      // Remove the message and never show it again.

I'd keep the original comment here ("Remove the message, so that it won't show again for the rest of the app lifetime.") as it's more accurate.

@@ +386,5 @@
> +    icon: "drawable://homepage_banner",
> +    onclick: function() {
> +      // Remove the message and never show it again.
> +      Home.banner.remove(id);
> +      Services.prefs.setBoolPref("browser.snippets.homepage.enabled", false);

Add a comment here stating that this will ensure we'll only show this banner once.

@@ +388,5 @@
> +      // Remove the message and never show it again.
> +      Home.banner.remove(id);
> +      Services.prefs.setBoolPref("browser.snippets.homepage.enabled", false);
> +
> +      UITelemetry.addEvent("action.1", "banner", null, "homepage");

I'm no UI telemetry expert but don't we want to differentiate between general snippets and this first-run hint somehow? Also, what would this specific UI telemetry data give us? It seems to me that knowing whether or not the user clicked/dismissed this first-run hint wouldn't be very useful. So maybe just don't add telemetry for this? Up to you.

::: mobile/android/locales/en-US/chrome/aboutHome.properties
@@ +1,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +banner.homepage.text=Welcome to your Homepage! Get back here every time you open a new tab.

This copy sounds slightly weird. 'Get back here' sounds like a sort of call for action. I'd expect this to sounds more like 'You'll see it every time you open a new tab.". Please double-check with UX folks.

Comment 33

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

> > +      // Remove the message and never show it again.
> > +      Home.banner.remove(id);
> > +      Services.prefs.setBoolPref("browser.snippets.homepage.enabled", false);
> > +
> > +      UITelemetry.addEvent("action.1", "banner", null, "homepage");
> 
> I'm no UI telemetry expert but don't we want to differentiate between
> general snippets and this first-run hint somehow? Also, what would this
> specific UI telemetry data give us? It seems to me that knowing whether or
> not the user clicked/dismissed this first-run hint wouldn't be very useful.
> So maybe just don't add telemetry for this? Up to you.

The Extra ("homepage") is how we have been differentiating snippets. In this case I think "homepage" might be too generic. Maybe "firstrun-homepage" might be better. Maybe change the preference too: "browser.snippets.firstrunHomepage.enabled" since we use camelCase for the existing "browser.snippets.syncPromo.enabled"

Comment 34

[Chenxia Liu [:liuche]]

> > +banner.homepage.text=Welcome to your Homepage! Get back here every time you open a new tab.
> 
> This copy sounds slightly weird. 'Get back here' sounds like a sort of call
> for action. I'd expect this to sounds more like 'You'll see it every time
> you open a new tab.". Please double-check with UX folks.

Anthony, any thoughts on this?

Comment 35

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

"Get back here" was actually intended to feel like an action because I have a sneaky suspicion that a part of the issue here is establishing a mental connection in the user's mind that "New tab" can show them so much more than just a URL bar to type in.

That being said, I think it's still too much hope to pin on a single string of copy so I'd be open to changing it :).

Let's try that for now and we can revisit the copy later cause I can't think of a much better one ATM and I don't want this to block it. :)

Comment 36

[Chenxia Liu [:liuche]]

Attachment: Part 1: Bounce animator v2

Addressed other comments from earlier feedback? request.
> 
> @@ +115,5 @@
> > +                @Override
> > +                public void run() {
> > +                    animator.start();
> > +                }
> > +            }, ANIMATION_DELAY_MS);
> 
> Why the delay?
> 

I think the delay is so that users will notice the animation more because it won't be happening with all the other startup motion (views appearing, etc).

Comment 37

[Chenxia Liu [:liuche]]

Attachment: Part 2: Home page snippet

Addressed comments, carrying over r+.

Comment 38

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

(In reply to Chenxia Liu [:liuche] from comment #36)
> I think the delay is so that users will notice the animation more because it
> won't be happening with all the other startup motion (views appearing, etc).

Yep! :D

Comment 39

[Lucas Rocha (:lucasr)]

Comment on attachment 8477020 [details] [diff] [review]
Part 1: Bounce animator v2

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

This looks good but I think you can simplify this patch by implementing a custom interpolator instead of the more complex bound animator (if OvershootInterpolator is not good enough). I don't feel strongly about it though.  Up to you if you want to go ahead with this approach or try implementing with an interpolator.

::: mobile/android/base/animation/BounceAnimator.java
@@ +17,5 @@
> + *
> + * After constructing an instance, animations can be queued up sequentially with the
> + * {@link #queue(Attributes) queue} method.
> + */
> +public class BounceAnimator extends ValueAnimator {

It seems to me that what you need is a custom Interpolator instead.

::: mobile/android/base/home/HomePagerTabStrip.java
@@ +58,5 @@
> +        final View nextTextView = getChildAt(getChildCount() - 1);
> +
> +        if (prevTextView == null || nextTextView == null) {
> +            return;
> +        }

nit: add empty line here.

@@ +63,5 @@
> +        // Set up initial values for the views that will be animated.
> +        ViewHelper.setTranslationX(prevTextView, -INIT_OFFSET);
> +        ViewHelper.setAlpha(prevTextView, 0);
> +        ViewHelper.setTranslationX(nextTextView, INIT_OFFSET);
> +        ViewHelper.setAlpha(nextTextView, 0);

Better not mix our framework with NineOldAndroids. Use ViewHelper from NineOldAndroid instead.

@@ +83,5 @@
> +        prevBounceAnimator.queue(new Attributes(-bounceDistance/4, BOUNCE2_MS));
> +        prevBounceAnimator.queue(new Attributes(0, BOUNCE4_MS));
> +        prevBounceAnimator.setStartDelay(ANIMATION_DELAY_MS);
> +
> +        final BounceAnimator nextBounceAnimator = new BounceAnimator(nextTextView, "translationX");

Just wondering: have you tried simply using an OvershootInterpolator instead of implementing your own bouncy animator? It would greatly simplify this patch.

Comment 40

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

(In reply to Lucas Rocha (:lucasr) from comment #39)

> > +        prevBounceAnimator.queue(new Attributes(-bounceDistance/4, BOUNCE2_MS));
> > +        prevBounceAnimator.queue(new Attributes(0, BOUNCE4_MS));
> > +        prevBounceAnimator.setStartDelay(ANIMATION_DELAY_MS);
> > +
> > +        final BounceAnimator nextBounceAnimator = new BounceAnimator(nextTextView, "translationX");
> 
> Just wondering: have you tried simply using an OvershootInterpolator instead
> of implementing your own bouncy animator? It would greatly simplify this
> patch.

Given the timelines, I'd consider looking at OvershootInterpolator in a follow up bug. Simplification would be better, but let's get this feature landed if the rest of the approach is OK.

Comment 41

[Chenxia Liu [:liuche]]

Filed follow-up bug 1057569. I tried a custom interpolator at some point, but it wasn't very extensible - it was pretty similar to the Android BounceInterpolator though, but I could try again coming up with a better equation.

https://hg.mozilla.org/integration/fx-team/rev/9d6bcb547b4d
https://hg.mozilla.org/integration/fx-team/rev/8da4c97261e9

Landed with bug 1056976.

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9d6bcb547b4d
https://hg.mozilla.org/mozilla-central/rev/8da4c97261e9

Comment 43

[:Margaret Leibovic]

Comment on attachment 8477021 [details] [diff] [review]
Part 2: Home page snippet

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

::: mobile/android/components/Snippets.js
@@ +416,1 @@
>            loadSyncPromoBanner();

Drive-by: We don't want the sync promo at all during the first run? Home banner messages rotate, so it is possible to have both.

Also, FYI, if the snippets update timer fires some time during the first run session, new snippets will get added to the rotation anyway (loadSnippetsFromCache doesn't actually do anything on first run, since there is no cache).

Comment 44

[Chenxia Liu [:liuche]]

Oh, I didn't realize that, actually. Filed bug 1058100.