align shipping option name to left and shipping cost to right on bottom sheet

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.payments.ui;
 
 import android.content.Context;
 import android.content.res.Resources;
 import android.graphics.Color;
 import android.graphics.Typeface;
@@ skipping 917 matching lines @@
                 int columnSpan = 1;
                 if (!optionIconExists) columnSpan++;
                 if (!editIconExists) columnSpan++;
 
                 TextView labelView = new TextView(context);
                 if (mRowType == OPTION_ROW_TYPE_OPTION) {
                     // Show the string representing the PaymentOption.
                     ApiCompatibilityUtils.setTextAppearance(labelView, isEnabled
                             ? R.style.PaymentsUiSectionDefaultText
                             : R.style.PaymentsUiSectionDisabledText);
-                    labelView.setText(convertOptionToString(mOption,
+                    labelView.setText(convertOptionToString(mOption, false, /* excludeMainLabel */
                             mDelegate.isBoldLabelNeeded(OptionSection.this),
                             false /* singleLine */));
                     labelView.setEnabled(isEnabled);
                 } else if (mRowType == OPTION_ROW_TYPE_ADD) {
                     // Shows string saying that the user can add a new option, e.g. credit card no.
                     String typeface = resources.getString(R.string.roboto_medium_typeface);
                     int textStyle = resources.getInteger(R.integer.roboto_medium_textstyle);
                     int buttonHeight = resources.getDimensionPixelSize(
                             R.dimen.payments_section_add_button_height);
 
@@ skipping 105 matching lines @@
         private SectionInformation mSectionInformation;
 
         /** Indicates whether the summary is displayed in a single line. */
         private boolean mSummaryInSingleLine;
 
         /** Indicates whether the summary is set to display in a single line in DISPLAY_MODE_NORMAL
          * by caller.
          */
         boolean mSetDisplaySummaryInSingleLineInNormalMode = true;
 
+        /** Indicates whether the summary should be split to display in left and right summary
+         * text views in {@link PaymentRequestSection.DISPLAY_MODE_NORMAL}. */

[gogerald1, 2017/06/02 15:05:10]

This is not a public interface or variable, so you can replace "{@link
PaymentRequestSection.DISPLAY_MODE_NORMAL}" with "DISPLAY_MODE_NORMAL".

In addition, if the comments have multiple lines, put '/**' and '*/' in a
separate line without any text in that line, like below.

/**
 * Indicates whether the summary should be split to display in the left and
right summary
 * text views in DISPLAY_MODE_NORMAL.
 */

[wuandy1, 2017/06/02 19:53:12]

Done.

+        private boolean mSplitSummaryInNormalMode;

[gogerald1, 2017/06/02 15:05:10]

looks 'mSplitSummaryInDisplayModeNormal' is better, 'DisplayModeNormal'
corresponds to 'DISPLAY_MODE_NORMAL'

[wuandy1, 2017/06/02 19:53:12]

Done.

+
         /** Indicates whether the summary is set to R.style.PaymentsUiSectionDescriptiveText. */
         private boolean mSummaryInDescriptiveText;
 
         private FocusChangedObserver mFocusChangedObserver;
 
         /**
          * Constructs an OptionSection.
          *
          * @param context     Context to pull resources from.
          * @param sectionName Title of the section to display.
@@ skipping 92 matching lines @@
 
         /**
          * @param singleLine If true, sets the summary text to display in a single line
          *                   in {@link #DISPLAY_MODE_NORMAL} when there is a valid selected
          *                   option, otherwise sets the summary text to display in multiple lines.
          */
         public void setDisplaySummaryInSingleLineInNormalMode(boolean singleLine) {
             mSetDisplaySummaryInSingleLineInNormalMode = singleLine;
         }
 
+        /**
+         * @param splitSummary set to true to split the display of summary in left and right

[gogerald1, 2017/06/02 15:05:10]

Uppercase the first character to comment a param. In addition, the your comments
is for the param but not the function, so "set to true to...." looks a little
confusing. You can follow below pattern.

/**
 * Function comments.....
 *
 * @param splitSummary If true, ....... Otherwise...
 */

[wuandy1, 2017/06/02 19:53:12]

Done.

+         *                     text views in {@link DISPLAY_MODE_NORMAL}. Otherwise the entire
+         *                     summary is displayed in left text view. Note that summary is
+         *                     split into 'label' and the rest('sublabel', 'Tertiary label').
+         */
+        public void setSplitSummaryInNormalMode(boolean splitSummary) {

[gogerald1, 2017/06/02 15:05:10]

setSplitSummaryInDisplayNormalMode?

[wuandy1, 2017/06/02 19:53:12]

Done.

+            mSplitSummaryInNormalMode = splitSummary;
+        }
+
         /** Updates the View to account for the new {@link SectionInformation} being passed in. */
         public void update(SectionInformation information) {
             mSectionInformation = information;
             PaymentOption selectedItem = information.getSelectedItem();
             updateSelectedItem(selectedItem);
             updateOptionList(information, selectedItem);
             updateControlLayout();
         }
 
         private View createLoadingSpinner() {
@@ skipping 97 matching lines @@
                         getContext(), mSectionInformation, getSummaryLeftTextView());
             } else {
                 setLogoDrawable(selectedItem.getDrawableIcon());
                 // Selected item summary should be displayed as
                 // R.style.PaymentsUiSectionDefaultText.
                 if (mSummaryInDescriptiveText) {
                     ApiCompatibilityUtils.setTextAppearance(
                             getSummaryLeftTextView(), R.style.PaymentsUiSectionDefaultText);
                     mSummaryInDescriptiveText = false;
                 }
-                setSummaryText(convertOptionToString(selectedItem, false /* useBoldLabel */,
-                                       mSummaryInSingleLine),
-                        null);
+                // Align main label to the left and sub label to the right if caller specified and
+                // section is in DISPLAY_MODE_NORMAL

[gogerald1, 2017/06/02 15:05:10]

// Split summary in DISPLAY_MODE_NORMAL if caller specified. The first part is
displayed on the left summary text view aligned to the left. The second part is
displayed on the right summary text view aligned to the right.

[wuandy1, 2017/06/02 19:53:12]

Done.

+                boolean splitSummary =
+                        mSplitSummaryInNormalMode && (mDisplayMode == DISPLAY_MODE_NORMAL);
+                if (splitSummary) {
+                    setSummaryText(selectedItem.getLabel(),
+                            convertOptionToString(selectedItem, true /* excludeMainLabel */,
+                                    false /* useBoldLabel */, mSummaryInSingleLine));
+                } else {
+                    setSummaryText(convertOptionToString(selectedItem, false /* excludeMainLabel */,
+                                           false /* useBoldLabel */, mSummaryInSingleLine),
+                            null);
+                }
             }
 
             updateControlLayout();
         }
 
         private void updateOptionList(SectionInformation information, PaymentOption selectedItem) {
             mOptionLayout.removeAllViews();
             mOptionRows.clear();
             mLabelsForTest.clear();
 
@@ skipping 34 matching lines @@
             // If the user is allowed to add new options, show the button for it.
             if (information.getAddStringId() != 0 && mCanAddItems) {
                 OptionRow addRow = new OptionRow(mOptionLayout, mOptionLayout.getChildCount(),
                         OptionRow.OPTION_ROW_TYPE_ADD, null, false);
                 addRow.setLabel(information.getAddStringId());
                 addRow.setButtonId(R.id.payments_add_option_button);
                 mOptionRows.add(addRow);
             }
         }
 
-        private CharSequence convertOptionToString(
-                PaymentOption item, boolean useBoldLabel, boolean singleLine) {
-            SpannableStringBuilder builder = new SpannableStringBuilder(item.getLabel());
-            String labelSeparator = singleLine
-                    ? getContext().getString(R.string.autofill_address_summary_separator)
-                    : "\n";
+        private CharSequence convertOptionToString(PaymentOption item, boolean excludeMainLabel,
+                boolean useBoldLabel, boolean singleLine) {
+            SpannableStringBuilder builder = new SpannableStringBuilder();
+            if (!excludeMainLabel) builder.append(item.getLabel());
+
             if (useBoldLabel) {

[gogerald1, 2017/06/02 15:05:10]

useBoldLabel is used for main label only

[wuandy1, 2017/06/02 19:53:12]

I think this is something better left to the callers to this function to decide.
This 
function as a utility should only provide the options available, while the
caller can
set useBoldLabel=false while excludeMainLabel=true accordingly.

[gogerald1, 2017/06/06 16:55:19]

Yes, but previously useBoldLabel is only apply for item.getLabel, we should keep
this fact and let caller do it or not. However after your change, the span will
be applied on an empty string sometimes, that doesn't make sense, so you might
want to move this logic inside the if(!excludeMainLabel) check.

[wuandy1, 2017/06/07 01:58:04]

absolutely! good catch!

                 builder.setSpan(
                         new StyleSpan(android.graphics.Typeface.BOLD), 0, builder.length(), 0);
             }
 
+            String labelSeparator = singleLine
+                    ? getContext().getString(R.string.autofill_address_summary_separator)
+                    : "\n";
             if (!TextUtils.isEmpty(item.getSublabel())) {
                 if (builder.length() > 0) builder.append(labelSeparator);
                 builder.append(item.getSublabel());
             }
 
             if (!TextUtils.isEmpty(item.getTertiaryLabel())) {
                 if (builder.length() > 0) builder.append(labelSeparator);
                 builder.append(item.getTertiaryLabel());
             }
 
@@ skipping 18 matching lines @@
          */
         @VisibleForTesting
         public TextView getOptionLabelsForTest(int labelIndex) {
             return mLabelsForTest.get(labelIndex);
         }
 
         /**
          * Returns the label of the section summary.
          */
         @VisibleForTesting
-        public TextView getSummaryLabelForTest() {
+        public TextView getLeftSummaryLabelForTest() {
             return getSummaryLeftTextView();
         }
 
+        /**
+         * Returns the label of the summary right text view for the OptionSection.

[gogerald1, 2017/06/02 15:05:10]

*the right summary text view* to be consistent

[wuandy1, 2017/06/02 19:53:12]

Done.

+         */
+        @VisibleForTesting
+        public TextView getRightSummaryLabelForTest() {
+            return getSummaryRightTextView();
+        }
+
         /** Returns the number of option labels. */
         @VisibleForTesting
         public int getNumberOfOptionLabelsForTest() {
             return mLabelsForTest.size();
         }
 
         /** Returns the OptionRow at the specified |index|. */
         @VisibleForTesting
         public OptionRow getOptionRowAtIndex(int index) {
             return mOptionRows.get(index);
@@ skipping 27 matching lines @@
         }
 
         /** Expand the separator to be the full width of the dialog. */
         public void expand() {
             LinearLayout.LayoutParams params = (LinearLayout.LayoutParams) getLayoutParams();
             ApiCompatibilityUtils.setMarginStart(params, 0);
             ApiCompatibilityUtils.setMarginEnd(params, 0);
         }
     }
 }