Adds animation as feature variation to keyboard accessory.

components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java

Index: components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java
diff --git a/components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java b/components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java
index 56475a385245b4eda771492457202070f1e3d3b9..a49aa799ff5ba02b3381bea30494d80b85738089 100644
--- a/components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java
+++ b/components/autofill/android/java/src/org/chromium/components/autofill/AutofillKeyboardAccessory.java
@@ -4,6 +4,9 @@
 
 package org.chromium.components.autofill;
 
+import android.animation.Animator;
+import android.animation.AnimatorListenerAdapter;
+import android.animation.ObjectAnimator;
 import android.annotation.SuppressLint;
 import android.graphics.Typeface;
 import android.os.Build;
@@ -29,19 +32,31 @@ import org.chromium.ui.base.WindowAndroid;
 public class AutofillKeyboardAccessory extends LinearLayout
         implements WindowAndroid.KeyboardVisibilityListener, View.OnClickListener,
         View.OnLongClickListener {
+    // We start animating by scrolling the suggestions from beyond the viewport.
+    private static final int NUM_DIP_TO_ANIMATE_SCROLL_FROM = 100;

[Ted C, 2017/05/17 00:04:30]

I would call this START_ANIMATION_TRANSLATION_X_DP = 100;

[csashi, 2017/05/17 01:14:15]

Done.

+
+    // Time to pause before reversing animation when the first suggestion is a hint.
+    private static final long PAUSE_ANIMATION_BEFORE_REVERSE_MILLIS = 1000;
+
     private final WindowAndroid mWindowAndroid;
     private final AutofillDelegate mAutofillDelegate;
+    // If |mMaximumLabelWidthPx| is 0, we do not call |setMaxWidth| on the |TextView| for a fillable
+    // suggestion label.
     private final int mMaximumLabelWidthPx;
     private final int mMaximumSublabelWidthPx;
+    private final int mAnimationDurationMillis;
+    private ObjectAnimator mAnimator;
 
     /**
      * Creates an AutofillKeyboardAccessory with specified parameters.
      * @param windowAndroid The owning WindowAndroid.
      * @param autofillDelegate A object that handles the calls to the native
      *                         AutofillKeyboardAccessoryView.
+     * @param animationDurationMillis If 0, do not animate.
+     * @param shouldLimitLabelWidth If true, limit suggestion label width to 1/2 device's width.
      */
-    public AutofillKeyboardAccessory(
-            WindowAndroid windowAndroid, AutofillDelegate autofillDelegate) {
+    public AutofillKeyboardAccessory(WindowAndroid windowAndroid, AutofillDelegate autofillDelegate,
+            int animationDurationMillis, boolean shouldLimitLabelWidth) {
         super(windowAndroid.getActivity().get());
         assert autofillDelegate != null;
         assert windowAndroid.getActivity().get() != null;
@@ -49,13 +64,15 @@ public class AutofillKeyboardAccessory extends LinearLayout
         mAutofillDelegate = autofillDelegate;
 
         int deviceWidthPx = windowAndroid.getDisplay().getDisplayWidth();
-        mMaximumLabelWidthPx = deviceWidthPx / 2;
+        mMaximumLabelWidthPx = shouldLimitLabelWidth ? deviceWidthPx / 2 : 0;
         mMaximumSublabelWidthPx = deviceWidthPx / 4;
 
         mWindowAndroid.addKeyboardVisibilityListener(this);
         int horizontalPaddingPx = getResources().getDimensionPixelSize(
                 R.dimen.keyboard_accessory_half_padding);
         setPadding(horizontalPaddingPx, 0, horizontalPaddingPx, 0);
+
+        mAnimationDurationMillis = animationDurationMillis;
     }
 
     /**
@@ -65,18 +82,28 @@ public class AutofillKeyboardAccessory extends LinearLayout
      */
     @SuppressLint("InlinedApi")
     public void showWithSuggestions(AutofillSuggestion[] suggestions, final boolean isRtl) {

[Ted C, 2017/05/17 00:04:30]

Is this called only a single time for each showing of keyboard suggestions? 
Just wondering if we need to worry about canceling animations or anything

[csashi, 2017/05/17 01:14:15]

It may be called again while previous animation in progress if I click on the
form element while animation is in progress - but that seems to work OK. It just
restarts the animation.

[csashi, 2017/05/18 00:04:05]

I am now trying a different variation to re-layout the views after animation is
over by removing the hint view. In that process, I added an animation cancel
also.

+        assert suggestions.length > 0;
+
         removeAllViews();
         int separatorPosition = -1;
         for (int i = 0; i < suggestions.length; i++) {
             AutofillSuggestion suggestion = suggestions[i];
-            assert !TextUtils.isEmpty(suggestion.getLabel());
+            // The first suggestion may be a hint to call attention to the keyboard accessory.
+            // See |IsHintEnabledInKeyboardAccessory|. A 'hint' suggestion does not have a label
+            // and is not fillable, but has an icon.
+            boolean isKeyboardAccessoryHint = i == 0 && TextUtils.isEmpty(suggestion.getLabel());
+            if (isKeyboardAccessoryHint) {
+                assert !suggestion.isFillable() && suggestion.getIconId() != 0;
+            } else {
+                assert !TextUtils.isEmpty(suggestion.getLabel());
+            }
 
             View touchTarget;
             if (!suggestion.isFillable() && suggestion.getIconId() != 0) {
                 touchTarget = LayoutInflater.from(getContext()).inflate(
                         R.layout.autofill_keyboard_accessory_icon, this, false);
 
-                if (separatorPosition == -1) separatorPosition = i;
+                if (separatorPosition == -1 && !isKeyboardAccessoryHint) separatorPosition = i;
 
                 ImageView icon = (ImageView) touchTarget;
                 icon.setImageDrawable(
@@ -89,7 +116,7 @@ public class AutofillKeyboardAccessory extends LinearLayout
                 TextView label = (TextView) touchTarget.findViewById(
                         R.id.autofill_keyboard_accessory_item_label);
 
-                if (suggestion.isFillable()) {
+                if (mMaximumLabelWidthPx > 0 && suggestion.isFillable()) {
                     label.setMaxWidth(mMaximumLabelWidthPx);
                 }
 
@@ -115,12 +142,13 @@ public class AutofillKeyboardAccessory extends LinearLayout
                 }
             }
 
-            touchTarget.setTag(i);
-            touchTarget.setOnClickListener(this);
-            if (suggestion.isDeletable()) {
-                touchTarget.setOnLongClickListener(this);
+            if (!isKeyboardAccessoryHint) {
+                touchTarget.setTag(i);
+                touchTarget.setOnClickListener(this);
+                if (suggestion.isDeletable()) {
+                    touchTarget.setOnLongClickListener(this);
+                }
             }
-
             addView(touchTarget);
         }
 
@@ -135,16 +163,59 @@ public class AutofillKeyboardAccessory extends LinearLayout
 
         final HorizontalScrollView container =
                 (HorizontalScrollView) mWindowAndroid.getKeyboardAccessoryView();
+        final boolean isFirstSuggestionAHint = TextUtils.isEmpty(suggestions[0].getLabel());
+        if (isFirstSuggestionAHint) {

[Ted C, 2017/05/17 00:04:30]

these asserts seem duplicated with what is in the loop above.

I wonder if we should just have

isFirstSuggestionAHint up top, then in the loop just do:

boolean isHint = i == 0;

To try and consolidate/de-duplicate the checks.

[csashi, 2017/05/17 01:14:15]

Done.

+            assert suggestions[0].getIconId() != 0 && !suggestions[0].isFillable();
+        }
+
         if (getParent() == null) {
             container.addView(this);
-            container.setVisibility(View.VISIBLE);
+            // If we are animating the view, we |setVisibility| in |onAnimationStart|.
+            if (mAnimationDurationMillis == 0) {
+                container.setVisibility(View.VISIBLE);
+            }
             container.sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_STATE_CHANGED);
         }
 
+        if (mAnimationDurationMillis > 0) {
+            // TODO(crbug/722897): Fix animation for RTL.

[Ted C, 2017/05/17 00:04:29]

Won't the suggestions just flow from the right to left in this case?

Just wondering why this isn't

int translateX = isRtl ? NUM_DIP_TO_ANIMATE_FROM : -NUM_DIP_TO_ANIMATE_FROM;

The hint will be on the right so you want it to peak out and hide.

[csashi, 2017/05/17 01:14:15]

I tried setting developer options to force right-to-left-layout direction but
the suggestion did not flow from right to left. I was not sure why the
setLayoutDirection View.LAYOUT_DIRECTION_RTL did not change this.

+            mAnimator = ObjectAnimator.ofFloat(
+                    this, View.TRANSLATION_X, -NUM_DIP_TO_ANIMATE_SCROLL_FROM, 0);

[Ted C, 2017/05/17 00:04:29]

translateX takes pixels, so despite your DIP in the name, you are always
animating 100 px regardless of density.

The easiest thing would be to put it in dimens.xml and read it from there. 
Otherwise, multiply it by context.getResources().getDisplayMetrics().density

[csashi, 2017/05/17 01:14:15]

Done.

+            mAnimator.setDuration(mAnimationDurationMillis);
+            mAnimator.addListener(new AnimatorListenerAdapter() {
+                @Override
+                public void onAnimationStart(Animator animator) {
+                    container.setVisibility(View.VISIBLE);
+                }
+                @Override
+                public void onAnimationEnd(Animator animator) {
+                    mAnimator.removeListener(this);
+                    if (isFirstSuggestionAHint) {

[Ted C, 2017/05/17 00:04:29]

if would do:

if (!isFirstSuggestionAHint) return;

Then the rest can be indented less.

+                        // Reverse the animation and scroll the first suggestion (which is a
+                        // non-fillable hint) out of the viewport at the end of the reversed
+                        // animation.
+                        assert getChildCount() > 1;
+                        int hintWidth = getChildAt(0).getWidth();
+                        // TODO(crbug/722897): Fix animation for RTL.
+                        mAnimator.setFloatValues(0, -hintWidth);

[Ted C, 2017/05/17 00:04:29]

I would just create a new animation.  The only thing we are getting is the
duration to be shared right?

[csashi, 2017/05/17 01:14:15]

Done.

+                        container.postDelayed(new Runnable() {
+                            @Override
+                            public void run() {
+                                mAnimator.start();

[Ted C, 2017/05/17 00:04:29]

You'll need to make sure you haven't been dismissed by the time this gets
called.

+                            }
+                        }, PAUSE_ANIMATION_BEFORE_REVERSE_MILLIS);
+                    }
+                }
+            });
+        }
         container.post(new Runnable() {
             @Override
             public void run() {
-                container.scrollTo(isRtl ? getRight() : 0, 0);
+                if (mAnimationDurationMillis > 0) {
+                    mAnimator.start();

[Ted C, 2017/05/17 00:04:29]

general question about this animation...translation doesn't normally affect
layout bounds.  scroll and translate do different things ("normally").

With you translating from off the screen onto screen.  I would assume the
suggestion on the far right (assuming LTR) would be clipped.  Then when you
translate the hint off the screen at the end of the process, I would think the
total drawable area for the autofill suggestions would be desired width -
hintWidth.

In the past when I've done these types of animations, I've need to "lie" to
android and tell them my total layout width was desired width + translation
delta.  Then once everything settles, you need to hide views/trigger
relayouts/do cleanups to get this all to fill the viewport nicely.  Maybe this
is all done for you by this view, which would be awesome.

[csashi, 2017/05/17 01:14:15]

This is an issue and the view does not take care of this. I am still trying to
tweak this. From the design doc:
"""
When we have a single card animating into and out of the view port, the settings
icon is no longer laid out at the right edge of the screen.
See https://drive.google.com/file/d/0B-jTnU8uuAL9NFN1cXU0Y3pmbXc/view.
"""

The solution I am working on is tweaking the width and layout weight for the
separator view. Please see createSeparatorView. It is not yet working correctly
and am wondering if for the purpose of the experiment we can simply not show the
last settings icon (I suspect it has very low CTR).

[csashi, 2017/05/18 00:04:05]

I did not have much success with tweaking the width and layout weight. Instead I
now remove the hint view after animation is over. It is still not working quite
right - I suspect there is one more padding I need to keep.

See https://drive.google.com/open?id=0B-jTnU8uuAL9SXI0NDc5bk9tNDg

[csashi, 2017/05/18 00:53:04]

I resolved this issue by resetting the 'X'. See
https://drive.google.com/open?id=0B-jTnU8uuAL9MjluS1oxbzlkM2s

+                } else {
+                    container.scrollTo(isRtl ? getRight() : 0, 0);
+                }
             }
         });
     }