Closed Bug 1021751 Opened 10 years ago Closed 10 years ago

Homepage contextual hint

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(fennec34+)

RESOLVED FIXED
Firefox 34
Tracking Status
fennec 34+ ---

People

(Reporter: ibarlow, Assigned: liuche)

References

Details

Attachments

(5 files, 7 obsolete files)

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.
OS: Mac OS X → Android
Hardware: x86 → ARM
Version: Firefox 31 → Trunk
tracking-fennec: --- → ?
(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?
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.
Anthony / Ian: How final are the mockups?
Assignee: nobody → liuche
Flags: needinfo?(alam)
tracking-fennec: ? → 33+
(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.
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.
Attached image prev_homepage_mock1.png (deleted) —
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.
Flags: needinfo?(alam)
Attached video onload_exp3.mov (deleted) —
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.
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).
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.
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.
Friendly ping on status here?
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.
Were you going to look into using reflection too?
Yep, that's what I'm looking at now.
This is obviously not going to make 33, so dropping that flag.
tracking-fennec: 33+ → ---
tracking-fennec: --- → ?
Attached patch Patch: Titlebar bounce (obsolete) (deleted) — Splinter Review
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.
Attachment #8460555 - Flags: review?(lucasr.at.mozilla)
Flags: needinfo?(alam)
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. :)
Flags: needinfo?(alam)
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?
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/
Attached patch Patch: WIP Homescreen contextual hint v2 (obsolete) (deleted) — Splinter Review
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.
Attachment #8460555 - Attachment is obsolete: true
Attachment #8460555 - Flags: review?(lucasr.at.mozilla)
Attachment #8460666 - Flags: feedback?(lucasr.at.mozilla)
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?
Flags: needinfo?(liuche)
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?
Attachment #8460666 - Flags: feedback?(lucasr.at.mozilla) → feedback+
tracking-fennec: ? → 34+
Anthony, a few more apks for you to try: http://people.mozilla.org/~liuche/bug-1021751/ See 5, 6A, and 6B.
Flags: needinfo?(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.
Flags: needinfo?(alam)
Attached patch Part 2-ish: BounceAnimator (obsolete) (deleted) — Splinter Review
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).
Attachment #8465964 - Flags: feedback?
Attached patch Part 3-ish: Homepage with BounceAnimator (obsolete) (deleted) — Splinter Review
This will eventually be merged with the Homescreen contextual hint patch, but is just a WIP demonstrating the use of the BounceAnimator.
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.
Attachment #8465964 - Flags: feedback?
Attached patch Part 3-ish: Homepage with BounceAnimator (obsolete) (deleted) — Splinter Review
(oops, uploaded a completely empty patch)
Attachment #8465965 - Attachment is obsolete: true
Attached file icon_home.zip (deleted) —
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.
Flags: needinfo?(alam)
Attached patch Part 1: Bounce animator (obsolete) (deleted) — Splinter Review
Bounce animator - need to fix gravity in the titlebar :/
Attachment #8460666 - Attachment is obsolete: true
Attachment #8465964 - Attachment is obsolete: true
Attachment #8466382 - Attachment is obsolete: true
Attached patch Part 2: Home page snippet (obsolete) (deleted) — Splinter Review
Attachment #8476396 - Flags: review?(lucasr.at.mozilla)
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.
Attachment #8476396 - Flags: review?(lucasr.at.mozilla) → review+
> > + // 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"
> > +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?
Blocks: 1056976
"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. :)
Attached patch Part 1: Bounce animator v2 (deleted) — Splinter Review
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).
Attachment #8476395 - Attachment is obsolete: true
Attachment #8477020 - Flags: review?(lucasr.at.mozilla)
Attached patch Part 2: Home page snippet (deleted) — Splinter Review
Addressed comments, carrying over r+.
Attachment #8476396 - Attachment is obsolete: true
Attachment #8477021 - Flags: review+
(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 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.
Attachment #8477020 - Flags: review?(lucasr.at.mozilla) → review+
(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.
Blocks: 1057569
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.
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 34
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).
Blocks: 1058100
Oh, I didn't realize that, actually. Filed bug 1058100.
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: