[Payments] Preview available payment options when no option is selected

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 491e92e00b182783e8264da764528c3ad3a5b6b3..b9d115154cdef206866d74446a102a6a992b0b0a 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
@@ -12,7 +12,9 @@ import android.graphics.drawable.Drawable;
 import android.os.Handler;
 import android.support.v4.view.animation.LinearOutSlowInInterpolator;
 import android.support.v7.widget.GridLayout;
+import android.text.Layout;
 import android.text.SpannableStringBuilder;
+import android.text.TextPaint;
 import android.text.TextUtils;
 import android.text.TextUtils.TruncateAt;
 import android.text.style.AbsoluteSizeSpan;
@@ -52,7 +54,7 @@ import javax.annotation.Nullable;
  * .................................................................|                |    or   .
  * . LEFT SUMMARY TEXT                        |  RIGHT SUMMARY TEXT |           LOGO |   ADD   .
  * .................................................................|                |    or   .
- * . MAIN SECTION CONTENT                                           |                |  SELECT .
+ * . MAIN SECTION CONTENT                                           |                |  CHOOSE .
  * .............................................................................................
  *
  * 1) MAIN CONTENT
@@ -64,9 +66,9 @@ import javax.annotation.Nullable;
  *    Displays an optional logo (e.g. a credit card image) that floats to the right of the main
  *    content.
  *
- * 3) CHEVRON or ADD or SELECT
+ * 3) CHEVRON or ADD or CHOOSE
  *    Drawn to indicate that the current section may be expanded.  Displayed only when the view is
- *    in the {@link #DISPLAY_MODE_EXPANDABLE} state and only if an ADD or SELECT button isn't shown.
+ *    in the {@link #DISPLAY_MODE_EXPANDABLE} state and only if an ADD or CHOOSE button isn't shown.
  *
  * There are three states that the UI may flip between; see {@link #DISPLAY_MODE_NORMAL},
  * {@link #DISPLAY_MODE_EXPANDABLE}, and {@link #DISPLAY_MODE_FOCUSED} for details.
@@ -110,7 +112,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
     public static final int EDIT_BUTTON_GONE = 0;
 
     /** Edit button mode: Indicate that the section requires a selection. */
-    public static final int EDIT_BUTTON_SELECT = 1;
+    public static final int EDIT_BUTTON_CHOOSE = 1;
 
     /** Edit button mode: Indicate that the section requires adding an option. */
     public static final int EDIT_BUTTON_ADD = 2;
@@ -210,6 +212,12 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
         return mSummaryRightTextView;
     }
 
+    /** Returns the left summary TextView. */
+    protected TextView getSummaryLeftTextView() {
+        assert mSummaryLeftTextView != null;
+        return mSummaryLeftTextView;
+    }
+
     @Override
     public boolean onInterceptTouchEvent(MotionEvent event) {
         // Allow touches to propagate to children only if the layout can be interacted with.
@@ -220,7 +228,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
     public final void onClick(View v) {
         if (!mDelegate.isAcceptingUserInput()) return;
 
-        // Handle clicking on "ADD" or "SELECT".
+        // Handle clicking on "ADD" or "CHOOSE".
         if (v == mEditButtonView) {
             if (getEditButtonState() == EDIT_BUTTON_ADD) {
                 mDelegate.onAddPaymentOption(this);
@@ -398,7 +406,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
     private Button createAndAddEditButton(ViewGroup parent) {
         Resources resources = parent.getResources();
         Button view = DualControlLayout.createButtonForLayout(
-                parent.getContext(), true, resources.getString(R.string.select), this);
+                parent.getContext(), true, resources.getString(R.string.choose), this);
         view.setId(R.id.payments_section);
 
         LayoutParams params =
@@ -451,22 +459,19 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
             mEditButtonView.setVisibility(GONE);
             mChevronView.setVisibility(
                     mDisplayMode == DISPLAY_MODE_EXPANDABLE ? VISIBLE : GONE);
-
-            // Update whether the summary is displayed.
-            boolean showSummary =
-                    mIsSummaryAllowed && !TextUtils.isEmpty(mSummaryLeftTextView.getText());
-            mSummaryLayout.setVisibility(showSummary ? VISIBLE : GONE);
         } else {
             // Show the edit button and hide the chevron and the summary.
             boolean isButtonAllowed = mDisplayMode == DISPLAY_MODE_EXPANDABLE
                     || mDisplayMode == DISPLAY_MODE_NORMAL;
-            mSummaryLayout.setVisibility(GONE);
             mChevronView.setVisibility(GONE);
             mEditButtonView.setVisibility(isButtonAllowed ? VISIBLE : GONE);
             mEditButtonView.setText(
-                    editButtonState == EDIT_BUTTON_SELECT ? R.string.select : R.string.add);
+                    editButtonState == EDIT_BUTTON_CHOOSE ? R.string.choose : R.string.add);
         }
 
+        // Update whether the summary is displayed.
+        mSummaryLayout.setVisibility(mIsSummaryAllowed ? VISIBLE : GONE);
+
         // The title gains extra spacing when there is another visible view in the main section.
         int numVisibleMainViews = 0;
         for (int i = 0; i < mMainSection.getChildCount(); i++) {
@@ -486,7 +491,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
     }
 
     /**
-     * Section with three extra TextViews beneath the summary to show additional details.
+     * Section with three extra TextViews beneath the summary to show additional shipping details.
      *
      * ............................................................................
      * . TITLE                                                          |         .
@@ -495,16 +500,17 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
      * .................................................................|    or   .
      * . EXTRA TEXT ONE                                                 |   ADD   .
      * .................................................................|    or   .
-     * . EXTRA TEXT TWO                                                 |  SELECT .
+     * . EXTRA TEXT TWO                                                 |  CHOOSE .
      * .................................................................|         .
      * . EXTRA TEXT THREE                                               |         .
      * ............................................................................
      */
-    public static class ExtraTextsSection extends PaymentRequestSection {
+    public static class ShippingSummarySection extends PaymentRequestSection {
         private TextView[] mExtraTextViews;
         private int mEditButtonState = EDIT_BUTTON_GONE;
 
-        public ExtraTextsSection(Context context, String sectionName, SectionDelegate delegate) {
+        public ShippingSummarySection(
+                Context context, String sectionName, SectionDelegate delegate) {
             super(context, sectionName, delegate);
             setExtraTexts(new String[] {null, null, null});
         }
@@ -525,11 +531,42 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
         }
 
         /**
+         * Updates the View to account for the new {@link ShippingSummaryInformation} being passed
+         * in.
+         */
+        public void update(ShippingSummaryInformation info) {
+            String selectedShippingName = info.getSelectedShippingAddressLabel();
+            String selectedShippingAddress = info.getSelectedShippingAddressSublabel();
+            String selectedShippingPhone = info.getSelectedShippingAddressTertiaryLabel();
+            String selectedShippingOptionLabel = info.getSelectedShippingOptionLabel();
+
+            // Display the summary in a single line.
+            setSummaryProperties(TruncateAt.END, true /* leftIsSingleLine */,
+                    null /* rightTruncate */, false /* rightIsSingleLine */);
+            if (selectedShippingAddress == null || selectedShippingOptionLabel == null) {
+                ApiCompatibilityUtils.setTextAppearance(
+                        getSummaryLeftTextView(), R.style.PaymentsUiSectionDescriptiveText);
+                SectionUiUtils.showSectionSummaryInTextViewInSingeLine(getContext(),
+                        info.getShippingAddressSectionInfo(), getSummaryLeftTextView());
+            } else {
+                // Show the shipping name in the summary section.
+                setSummaryText(selectedShippingName, null);
+
+                // Show the shipping address, phone and option below the summary.
+                setExtraTextsProperties(
+                        new TruncateAt[] {TruncateAt.MIDDLE, TruncateAt.END, TruncateAt.END},
+                        new boolean[] {true, true, true});
+                setExtraTexts(new String[] {selectedShippingAddress, selectedShippingPhone,
+                        selectedShippingOptionLabel});
+            }
+        }
+
+        /**
          * Sets the CharSequences that are displayed in the extra TextViews.
          *
          * @param extraTexts Texts to display in the extra TextViews.
          */
-        public void setExtraTexts(CharSequence[] extraTexts) {
+        private void setExtraTexts(CharSequence[] extraTexts) {
             assert extraTexts.length == mExtraTextViews.length;
 
             for (int i = 0; i < mExtraTextViews.length; i++) {
@@ -544,7 +581,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
          * @param textsTruncate How to truncate the extra texts. Set the element to null to clear.
          * @param textsAreSingleLine Whether the extra texts should be displayed in a single line.
          */
-        public void setExtraTextsProperties(
+        private void setExtraTextsProperties(
                 TruncateAt[] textsTruncate, boolean[] textsAreSingleLine) {
             assert textsTruncate.length == mExtraTextViews.length;
             assert textsAreSingleLine.length == mExtraTextViews.length;
@@ -581,7 +618,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
      * . LEFT SUMMARY TEXT          | UPDATE TEXT |  RIGHT SUMMARY TEXT |    or   .
      * .................................................................|   ADD   .
      * .                                      | Line item 1 |    $13.99 |    or   .
-     * .                                      | Line item 2 |      $.99 |  SELECT .
+     * .                                      | Line item 2 |      $.99 |  CHOOSE .
      * .                                      | Line item 3 |     $2.99 |         .
      * ............................................................................
      */
@@ -826,7 +863,7 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
      * . Descriptive text that spans all three columns because it can.  |                |    or   .
      * . ! Warning text that displays a big scary warning and icon.     |           LOGO |   ADD   .
      * . O Option 1                                  ICON 1 | Edit Icon |                |    or   .
-     * . O Option 2                                  ICON 2 | Edit Icon |                |  SELECT .
+     * . O Option 2                                  ICON 2 | Edit Icon |                |  CHOOSE .
      * . O Option 3                                  ICON 3 | Edit Icon |                |         .
      * . + ADD THING                                                    |                |         .
      * .............................................................................................
@@ -894,7 +931,6 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
 
                 ((RadioButton) mButton).setChecked(isChecked);
                 if (isChecked) {
-                    updateSelectedItem(mOption);
                     mDelegate.onPaymentOptionChanged(OptionSection.this, mOption);
                 }
             }
@@ -1116,6 +1152,9 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
         /** Indicates whether the summary should be a single line. */
         private boolean mSummaryInSingleLine;
 
+        /** Indicates whether the summary is set to R.style.PaymentsUiSectionDescriptiveText. */
+        private boolean mSummaryInDescriptiveText;
+
         private FocusChangedObserver mFocusChangedObserver;
 
         /**
@@ -1186,7 +1225,13 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
                         mSectionInformation.getDataType(), shouldFocus);
             }
 
+            int previousDisplayMode = mDisplayMode;
             super.focusSection(shouldFocus);
+
+            // Update summary when display mode changed from DISPLAY_MODE_NORMAL to other modes.
+            if (mSectionInformation != null && previousDisplayMode == DISPLAY_MODE_NORMAL) {
+                updateSelectedItem(mSectionInformation.getSelectedItem());
+            }
         }
 
         @Override
@@ -1195,20 +1240,6 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
         }
 
         @Override
-        public void setSummaryProperties(@Nullable TruncateAt leftTruncate,
-                boolean leftIsSingleLine, @Nullable TruncateAt rightTruncate,
-                boolean rightIsSingleLine) {
-            super.setSummaryProperties(
-                    leftTruncate, leftIsSingleLine, rightTruncate, rightIsSingleLine);
-
-            // Updates the summary if necessary after properties are changed.
-            mSummaryInSingleLine = leftIsSingleLine;
-            if (mSectionInformation != null) {
-                updateSelectedItem(mSectionInformation.getSelectedItem());
-            }
-        }
-
-        @Override
         protected void createMainSectionContent(LinearLayout mainSectionLayout) {
             Context context = mainSectionLayout.getContext();
             mCheckingProgress = createLoadingSpinner();
@@ -1296,19 +1327,40 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
                 return EDIT_BUTTON_ADD;
             } else if (mSectionInformation.getSelectedItem() == null) {
                 // The user hasn't selected any available PaymentOptions.  Ask the user to pick one.
-                return EDIT_BUTTON_SELECT;
+                return EDIT_BUTTON_CHOOSE;
             } else {
                 return EDIT_BUTTON_GONE;
             }
         }
 
         private void updateSelectedItem(PaymentOption selectedItem) {
+            // Display summary in a single line if no selected item or in DISPLAY_MODE_NORMAL.
+            if (selectedItem == null || mDisplayMode == DISPLAY_MODE_NORMAL) {
+                if (!mSummaryInSingleLine) {
+                    setSummaryProperties(TruncateAt.END, true /* leftIsSingleLine */,
+                            null /* rightTruncate */, false /* rightIsSingleLine */);

[gone, 2017/03/28 00:20:38]

this is set repeatedly in the same way, but in different places; should you just
have some sort of helper function that does this and explains in better detail
what you're doing?

[gogerald1, 2017/03/28 18:46:24]

Added more explains, not sure what helper function would make it better

+                    mSummaryInSingleLine = true;
+                }
+            } else if (mSummaryInSingleLine) {
+                setSummaryProperties(null /* leftTruncate */, false /* leftIsSingleLine */,
+                        null /* rightTruncate */, false /* rightIsSingleLine */);
+                mSummaryInSingleLine = false;
+            }
+
             if (selectedItem == null) {
                 setLogoDrawable(null, false);
-                setIsSummaryAllowed(false);
-                setSummaryText(null, null);
+                if (!mSummaryInDescriptiveText) {
+                    ApiCompatibilityUtils.setTextAppearance(
+                            getSummaryLeftTextView(), R.style.PaymentsUiSectionDescriptiveText);
+                }
+                SectionUiUtils.showSectionSummaryInTextViewInSingeLine(
+                        getContext(), mSectionInformation, getSummaryLeftTextView());
             } else {
                 setLogoDrawable(selectedItem.getDrawableIcon(), selectedItem.isEditable());
+                if (mSummaryInDescriptiveText) {
+                    ApiCompatibilityUtils.setTextAppearance(
+                            getSummaryLeftTextView(), R.style.PaymentsUiSectionDefaultText);
+                }
                 setSummaryText(
                         convertOptionToString(selectedItem, false /* useBoldLabel */,
                                 mSummaryInSingleLine),
@@ -1458,4 +1510,105 @@ public abstract class PaymentRequestSection extends LinearLayout implements View
             ApiCompatibilityUtils.setMarginEnd(params, 0);
         }
     }
+
+    private static class SectionUiUtils {

[gone, 2017/03/28 00:20:39]

Should pull this out into its own file.  It's standalone and the file is already
1500 lines long.  That'd let you test the eliding algorithm directly, too.

[gogerald1, 2017/03/28 18:46:23]

Done.

+        /** Avoid instantiation by accident. */
+        private SectionUiUtils() {}
+
+        /**
+         * Show the section summary in the view in a single line.
+         *
+         * @param context The context.
+         * @param section The section to summarize.
+         * @param view    The view to display the summary.
+         */
+        public static void showSectionSummaryInTextViewInSingeLine(
+                final Context context, final SectionInformation section, final TextView view) {
+            int optionCount = section.getSize();
+            if (optionCount == 0) {
+                view.setText(null);
+                return;
+            }
+
+            // Listen for layout event to check whether the string can be fit in a single line and
+            // manually ellide the string in the middle if necessary.

[gone, 2017/03/28 00:20:38]

elide

[gogerald1, 2017/03/28 18:46:23]

Done.

+            if (view.getLayout() == null && optionCount > 1) {
+                view.addOnLayoutChangeListener(new View.OnLayoutChangeListener() {
+                    @Override
+                    public void onLayoutChange(View v, int left, int top, int right, int bottom,
+                            int oldLeft, int oldTop, int oldRight, int oldBottom) {
+                        view.removeOnLayoutChangeListener(this);

[gone, 2017/03/28 00:20:38]

what happens if the screen is rotated or the screen is split in half (android
N+)?  should you keep listening in that case?

[gogerald1, 2017/03/28 18:46:23]

Done.

+                        if (section.getSelectedItem() != null) return;
+
+                        Layout layout = view.getLayout();
+                        if (layout.getEllipsisCount(0) > 0) {
+                            String summary = getSectionSummaryForPreviewInASingleLine(
+                                    context, section, layout, view.getPaint());
+                            view.setText(summary);
+                        }
+                    }
+                });
+            }
+
+            String summary = SectionUiUtils.getSectionSummaryForPreviewInASingleLine(
+                    context, section, view.getLayout(), view.getPaint());
+            view.setText(summary);
+        }
+
+        private static String getSectionSummaryForPreviewInASingleLine(Context context,

[gone, 2017/03/28 00:20:38]

Explain your shrinking algorithm here.  It's a mysterious block of code
otherwise.

[gogerald1, 2017/03/28 18:46:23]

Done.

+                SectionInformation section, @Nullable Layout layout, @Nullable TextPaint paint) {
+            int optionCount = section.getSize();
+            assert optionCount != 0;
+
+            PaymentOption option = section.getItem(0);
+            String optionSummary = convertOptionToPreviewString(option, context);
+            int moreOptionCount = optionCount - 1;
+            if (moreOptionCount == 0) return optionSummary;
+
+            // Append string to indicate how many more options in the given section.

[gone, 2017/03/28 00:20:38]

Did you talk with srahim@ like I asked?  You can't always just concatenate two
strings and expect it to be grammatically correct.

[gogerald1, 2017/03/28 18:46:24]

I've talked to bbergher@ before who is in the same team as srahim@. Didn't see
any problem.
Mentioned your concern again in the team meeting. bbergher@ will double confirm
it.
I've put a resource string in payments_strings.grdp for each payment section,
should be much easier for translation,

[gone, 2017/03/28 18:47:22]

srahim@ was specifically hired for strings.

+            int resId = section.getPreviewStringResourceId();
+            assert resId > 0;
+            if (paint == null || layout == null) {
+                return context.getString(resId, optionSummary, moreOptionCount);
+            }
+
+            int ellipsizedWidth = layout.getEllipsizedWidth();
+            if (option.isPaymentCard()) {

[please use gerrit instead, 2017/03/27 20:28:31]

Instead of switching on option.isPaymentCard(), define a
getSummary(Context,int,..) method in PaymentOption.java or
PaymentInstrument.java with the default summary that you have on lines
1587-1591. Then override getSummary() in AutofillPaymentInstrument.java using
the code that you have on lines 1577-1584 from this file. Then please remove
isPaymentCard().

[gogerald1, 2017/03/27 23:26:47]

Done.

+                float desiredWidth = Layout.getDesiredWidth(
+                        context.getString(resId, optionSummary, moreOptionCount), paint);
+                if (desiredWidth > ellipsizedWidth) {
+                    // Only display network name separated by space from card number.
+                    optionSummary = optionSummary.substring(0, optionSummary.indexOf(" "));
+                }
+
+                return context.getString(resId, optionSummary, moreOptionCount);
+            }
+
+            String summary = context.getString(resId, optionSummary, moreOptionCount);
+            while (Layout.getDesiredWidth(summary, paint) > ellipsizedWidth) {
+                optionSummary = optionSummary.substring(0, optionSummary.length() / 2);
+                summary = context.getString(resId, optionSummary, moreOptionCount);
+            }
+
+            return summary;
+        }
+
+        private static String convertOptionToPreviewString(PaymentOption option, Context context) {
+            String previewString = option.getLabel();
+            if (option.isPaymentCard()) return previewString;

[please use gerrit instead, 2017/03/27 20:28:30]

Define default getPreviewString(Context) in PaymentOption.java or
PaymentInstrument.java. Override it with custom implementation in
AutofillPaymentInstrument.java. Then please remove isPaymentCard().

[gogerald1, 2017/03/27 23:26:47]

Done.

+
+            String labelSeparator = context.getString(R.string.autofill_address_summary_separator);
+            if (!TextUtils.isEmpty(option.getSublabel())) {
+                if (previewString.length() > 0) previewString += labelSeparator;

[please use gerrit instead, 2017/03/27 20:28:30]

Does this work in RTL?

[gogerald1, 2017/03/27 23:26:47]

Yes, words order in a string doesn't change in RTL,

+                previewString += option.getSublabel();
+            }
+
+            if (!TextUtils.isEmpty(option.getTertiaryLabel())) {
+                if (previewString.length() > 0) previewString += labelSeparator;
+                previewString += option.getSublabel();
+            }
+
+            return previewString;
+        }
+    }
 }