[Payments] Add text when shopping card total is updated.

chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java

Index: chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
index 4fe16f77762f31c6ed2a0c10ed2b9141ce998cd7..6e0ef2ea06ff0516d6415fd985d3248be94cd78e 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
@@ -9,6 +9,8 @@ import android.content.res.Resources;
 import android.graphics.Color;
 import android.graphics.Typeface;
 import android.graphics.drawable.Drawable;
+import android.os.Handler;
+import android.support.v4.view.animation.LinearOutSlowInInterpolator;
 import android.text.SpannableStringBuilder;
 import android.text.TextUtils;
 import android.text.TextUtils.TruncateAt;
@@ -20,6 +22,8 @@ import android.view.LayoutInflater;
 import android.view.MotionEvent;
 import android.view.View;
 import android.view.ViewGroup;
+import android.view.animation.AlphaAnimation;
+import android.view.animation.Animation;
 import android.widget.Button;
 import android.widget.GridLayout;
 import android.widget.ImageButton;
@@ -46,15 +50,16 @@ import javax.annotation.Nullable;
  * .............................................................................................
  * . TITLE                                                          |                | CHEVRON .
  * .................................................................|                |    or   .
- * . LEFT SUMMARY TEXT                        |  RIGHT SUMMARY TEXT |           LOGO |   ADD   .
+ * . LEFT SUMMARY TEXT | OPTIONAL UPDATE TEXT |  RIGHT SUMMARY TEXT |           LOGO |   ADD   .
  * .................................................................|                |    or   .
  * . MAIN SECTION CONTENT                                           |                |  SELECT .
  * .............................................................................................
  *
  * 1) MAIN CONTENT
  *    The main content is on the left side of the UI.  This includes the title of the section and
- *    two bits of optional summary text.  Subclasses may extend this class to append more controls
- *    via the {@link #createMainSectionContent} function.
+ *    two bits of optional summary text.  A text view to indicate an update can optionnaly be added
+ *    between these two summary texts.  Subclasses may extend this class to append more controls via
+ *    the {@link #createMainSectionContent} function.
  *
  * 2) LOGO
  *    Displays an optional logo (e.g. a credit card image) that floats to the right of the main
@@ -120,6 +125,12 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
     /** Checking mode: Gray background, spinner overlay hides everything except the title. */
     static final int DISPLAY_MODE_CHECKING = 6;
 
+    /** The duration of the animation to show and hide the update text. */
+    static final int UPDATE_TEXT_ANIMATION_DURATION_MS = 500;
+
+    /** The amount of time where the update text is visible before fading out. */
+    static final int UPDATE_TEXT_VISIBILITY_DURATION_MS = 5000;
+
     protected final SectionDelegate mDelegate;
     protected final int mLargeSpacing;
     protected final Button mEditButtonView;
@@ -127,6 +138,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
 
     protected int mDisplayMode = DISPLAY_MODE_NORMAL;
 
+    private final Handler mHandler = new Handler();
     private final int mVerticalSpacing;
     private final int mFocusedBackgroundColor;
     private final LinearLayout mMainSection;
@@ -137,10 +149,14 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
     private LinearLayout mSummaryLayout;
     private TextView mSummaryLeftTextView;
     private TextView mSummaryRightTextView;
+    private TextView mUpdatedView;
 
     private Drawable mLogo;
     private boolean mIsSummaryAllowed = true;
 
+    /** The number of animation that are waiting to fade out. */
+    private int mPendingAnimations;
+
     /**
      * Constructs a PaymentRequestSection.
      *
@@ -243,6 +259,54 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
     }
 
     /**
+     * Show the update text if the right text of the view has changed. Should be called before
+     * changing the text value.
+     *
+     * @param rightText Text to display on the right side.

[Ted C, 2016/11/16 19:29:40]

Instead of "Text to display", I would say the "Text displayed".  I was
incorrectly thinking this was the update text itself, but reading it more
closely, it is the text such as cost (or whatever) where we want to tell the
user it updated.

[sebsg, 2016/11/17 20:14:39]

Oh I see the misunderstanding. But I don't think "Text displayed" works either
because it is not the text that is currently displayed. It's the text that will
be displayed next. Is "The new cart total that will replace the one currently
displayed." better?

+     */
+    public void showUpdateIfTextChanged(@Nullable CharSequence rightText) {

[Ted C, 2016/11/16 19:29:40]

looks like this could be private as well?

[sebsg, 2016/11/17 20:14:39]

It has to be public to be available to the nested classes :S

Edit: It can now be private in the child class. thanks!

+        // This should only be called for a section that has an update text.
+        assert mUpdatedView != null;
+
+        // If either the old or new text was null do nothing.
+        if (rightText == null || mSummaryRightTextView.getText() == null) return;
+
+        // Show the update text only if the old and new versions of the right text are different and
+        // the old text was visible to the user.
+        if (!mSummaryRightTextView.getText().toString().equals(rightText.toString())

[Ted C, 2016/11/16 19:29:40]

I would use TextUtils.equals(...) here

[sebsg, 2016/11/17 20:14:39]

Done.

+                && mSummaryRightTextView.getVisibility() == VISIBLE) {
+            startUpdateViewAnimation();
+        }
+    }
+
+    /*

[Ted C, 2016/11/16 19:29:40]

add another * to make this a javadoc.

[sebsg, 2016/11/17 20:14:39]

Done.

+     * Starts the animation to make the update text view fade in then fade out.
+     */
+    private void startUpdateViewAnimation() {
+        Animation in = new AlphaAnimation(0.0f, 1.0f);

[Ted C, 2016/11/16 19:29:40]

could we use mUpdateView.getAlpha() instead of 0f?

Same in the fade out logic below.

That way if you were in the middle of fading out, then a fade in would start at
the correct value.

[sebsg, 2016/11/17 20:14:39]

I really agree for the fade out. But I'm not sure for the fade in. I kinda like
that it starts over if you make a change while the text is still there. Makes it
feel reactive to me, that it acknoledges the change I just made. That being
said, I'm no UI expert at all so I'll defer to you.

+        in.setDuration(UPDATE_TEXT_ANIMATION_DURATION_MS);
+        in.setInterpolator(new LinearOutSlowInInterpolator());
+        mUpdatedView.setAlpha(1.0f);

[Ted C, 2016/11/16 19:29:40]

why the need to set the alpha to 1 here?

[sebsg, 2016/11/17 20:14:39]

It was because I set the alpha initially to 0 and even with the
setFillAfter(true) it did not stay visible. But I found a cleaner way. Thanks
for the comment.

+        mUpdatedView.startAnimation(in);
+        ++mPendingAnimations;
+
+        mHandler.postDelayed(new Runnable() {

[Ted C, 2016/11/16 19:29:40]

If you were to add an AnimationListener to the Animation, would onAnimationEnd
be called when you are overwriting the animations from subsequent calls?

https://developer.android.com/reference/android/view/animation/Animation.Anim...

In particular, that would be nicer than posting tasks.  If the listener doesn't
work, you could pull out a Runnable.

private Runnable mFadeOutRunnable = new Runnable() {...};

Then you could do:

mHandler.removeCallbacks(mFadeOutRunnable);
mHandler.postDelayed(mFadeOutRunnable, UPDATE_TEXT_VISIBILITY_DURATION_MS);

Then you wouldn't need the pendingAnimationCount.

The Handler also poses an issue as it will run after this class has gone away,
so you could be posting tasks that are run when the view is detached from the
view hierarchy, so it is another reason to use the listener if you can.

[sebsg, 2016/11/17 20:14:39]

But I would still have to post a task, no? Because I still have to wait for
UPDATE_TEXT_VISIBILITY_DURATION_MS after the call to the Listener's
onAnimationStart/End?

I implemented your second suggestions in the meantime :) thanks!

[Ted C, 2016/11/17 23:30:55]

Ah yes, I didn't see the two times were the same.  Indeed, you'll need a handler
then.

+            @Override
+            public void run() {
+                --mPendingAnimations;
+                // Only fade out if it was the last animation.
+                if (mPendingAnimations == 0) {
+                    Animation out = new AlphaAnimation(1.0f, 0.0f);
+                    out.setDuration(UPDATE_TEXT_ANIMATION_DURATION_MS);
+                    out.setInterpolator(new LinearOutSlowInInterpolator());
+                    out.setFillAfter(true);
+                    mUpdatedView.startAnimation(out);
+                }
+            }
+        }, UPDATE_TEXT_VISIBILITY_DURATION_MS);
+    }
+
+    /**
      * Sets how the summary text should be displayed.
      *
      * @param leftTruncate How to truncate the left summary text.  Set to null to clear.
@@ -341,7 +405,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
                 getContext().getResources().getDimensionPixelSize(
                         R.dimen.payments_section_small_spacing));
 
-        // The summary section displays up to two TextViews side by side.
+        // The summary section displays up to three TextViews side by side.
         mSummaryLayout = new LinearLayout(getContext());
         mSummaryLayout.addView(mSummaryLeftTextView, leftLayoutParams);
         mSummaryLayout.addView(mSummaryRightTextView, rightLayoutParams);
@@ -353,6 +417,35 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
         return mainSectionLayout;
     }
 
+    /**
+     * Adds a text view to the summary layout that will be used to indicate that the section has
+     * been updated.
+     *
+     * @param text The text to be shown when the right summary label text of the section changes.
+     */
+    public void addUpdateText(String text) {

[Ted C, 2016/11/16 19:29:40]

all the other methods that do the same seem to be private.  any reason for this
to be public?

[sebsg, 2016/11/17 20:14:39]

The had to be public to be accessible in the base class because the child was
nested. It is now private in the child itself.

+        mUpdatedView = new TextView(getContext());

[Ted C, 2016/11/16 19:29:40]

maybe add an assert that mUpdatedView == null before this?

[sebsg, 2016/11/17 20:14:39]

Done.

+        mUpdatedView.setText(text);
+        ApiCompatibilityUtils.setTextAppearance(mUpdatedView, R.style.PaymentsUiSectionDefaultText);
+
+        // Set the text to initially be invisible.
+        mUpdatedView.setAlpha(0.0f);
+
+        LinearLayout.LayoutParams updatedLayoutParams =
+                new LinearLayout.LayoutParams(0, LayoutParams.WRAP_CONTENT);
+        updatedLayoutParams.weight = 1;
+        ApiCompatibilityUtils.setTextAlignment(mUpdatedView, TEXT_ALIGNMENT_TEXT_END);
+        mUpdatedView.setTextColor(ApiCompatibilityUtils.getColor(
+                getContext().getResources(), R.color.payments_updated_text_color));
+        ApiCompatibilityUtils.setMarginEnd(
+                updatedLayoutParams, getContext().getResources().getDimensionPixelSize(
+                                             R.dimen.payments_section_small_spacing));
+
+        // Add the update text just before the last summary text.
+        mSummaryLayout.addView(
+                mUpdatedView, mSummaryLayout.getChildCount() - 1, updatedLayoutParams);
+    }
+
     private static ImageView createAndAddLogoView(
             ViewGroup parent, int resourceId, int startMargin) {
         ImageView view = new ImageView(parent.getContext());
@@ -551,7 +644,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
      * ............................................................................
      * . TITLE                                                          |         .
      * .................................................................| CHERVON .
-     * . LEFT SUMMARY TEXT                        |  RIGHT SUMMARY TEXT |    or   .
+     * . LEFT SUMMARY TEXT          | UPDATE TEXT |  RIGHT SUMMARY TEXT |    or   .
      * .................................................................|   ADD   .
      * .                                      | Line item 1 |    $13.99 |    or   .
      * .                                      | Line item 2 |      $.99 |  SELECT .
@@ -562,8 +655,11 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
         private GridLayout mBreakdownLayout;
 
         public LineItemBreakdownSection(
-                Context context, String sectionName, SectionDelegate delegate) {
+                Context context, String sectionName, SectionDelegate delegate, String updatedText) {
             super(context, sectionName, delegate);
+
+            // Add a label that will be used to indicate that the total has been updated.
+            addUpdateText(updatedText);

[Ted C, 2016/11/16 19:29:40]

dfalcantara@ can probably better speak to this than me, but it looks like this
should be done in createMainSectionContent.

Then showUpdateIfTextChanged would exist within this class.  To me, it seems
similar to ExtraTextsSection and it's exposed setExtraTexts stuff.

[gone, 2016/11/16 19:57:28]

Yeah, this belongs in createMainSectionContent with the other controls that are
created for this section.  If you don't put it there, it gets increasingly
harder to determine the order that controls will be added to the layout.

[sebsg, 2016/11/17 20:14:39]

Done.

         }
 
         @Override
@@ -588,9 +684,14 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
         public void update(ShoppingCart cart) {
             Context context = mBreakdownLayout.getContext();
 
+            CharSequence totalPrice = createValueString(
+                    cart.getTotal().getCurrency(), cart.getTotal().getPrice(), true);
+
+            // Show the updated text view if the total changed.
+            showUpdateIfTextChanged(totalPrice);
+
             // Update the summary to display information about the total.
-            setSummaryText(cart.getTotal().getLabel(), createValueString(
-                    cart.getTotal().getCurrency(), cart.getTotal().getPrice(), true));
+            setSummaryText(cart.getTotal().getLabel(), totalPrice);
 
             mBreakdownLayout.removeAllViews();
             if (cart.getContents() == null) return;