[Autofill Assistant] Increased touch targets in PR to min. 48dp. Fixed privacy notice alignment.

This CL adjusts margins and paddings such that all touchable items in the autofill assistant payment request have a minimum size of 48dp*48dp. Where possible, margins were reduced and paddings were added to keep the same visual appearance with larger touch targets.

Also, the privacy notice is now left and right-aligned with the rest of the payment request.

Before: https://screenshot.googleplex.com/9q4uqGsqKnp.png
After: https://screenshot.googleplex.com/tbjwR2sCxrj.png

Touch targets before: https://screenshot.googleplex.com/OfaTnJGm3ty.png
Touch targets after: https://screenshot.googleplex.com/DPq1Eovc21K.png

Bug: b/135214309
Change-Id: I0ef92c7e5442bde35f6a639c94add9f3ee8ace46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670220
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672452}

chrome/android/features/autofill_assistant/java/res/layout/autofill_assistant_address_full.xml

@@ -7,7 +7,8 @@
               android:id="@+id/address_full"
               android:layout_width="match_parent"
               android:layout_height="wrap_content"
-              android:orientation="vertical">
+              android:orientation="vertical"
+              android:gravity="center_vertical">
 
     <TextView android:id="@+id/full_name"
               android:layout_width="wrap_content"

chrome/android/features/autofill_assistant/java/res/layout/autofill_assistant_contact_full.xml

@@ -6,7 +6,8 @@
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
               android:layout_width="match_parent"
               android:layout_height="wrap_content"
-              android:orientation="vertical">
+              android:orientation="vertical"
+              android:gravity="center_vertical">
     <TextView
         android:id="@+id/contact_full"
         android:layout_width="match_parent"

chrome/android/features/autofill_assistant/java/res/layout/autofill_assistant_payment_method_full.xml

@@ -8,7 +8,8 @@
     android:id="@+id/payment_method_full"
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
-    android:orientation="vertical">
+    android:orientation="vertical"
+    android:gravity="center_vertical">
 
     <LinearLayout
         android:layout_width="match_parent"

chrome/android/features/autofill_assistant/java/res/layout/autofill_assistant_payment_request_choice_list.xml

@@ -14,8 +14,7 @@
     android:background="@color/payments_section_edit_background"
     android:paddingStart="@dimen/autofill_assistant_bottombar_horizontal_spacing"
     android:paddingTop="8dp"
-    android:paddingEnd="@dimen/autofill_assistant_bottombar_horizontal_spacing"
+    android:paddingEnd="@dimen/autofill_assistant_payment_request_choice_list_padding_end"
     android:paddingBottom="8dp"
-    app:column_spacing="8dp"
-    app:row_spacing="8dp">
+    app:column_spacing="8dp">
 </org.chromium.chrome.browser.autofill_assistant.payment.AssistantChoiceList>
\ No newline at end of file

chrome/android/features/autofill_assistant/java/res/layout/autofill_assistant_payment_request_terms_and_conditions.xml

@@ -17,7 +17,6 @@
         android:layout_marginStart="@dimen/autofill_assistant_bottombar_horizontal_spacing"
         android:layout_marginEnd="@dimen/autofill_assistant_bottombar_horizontal_spacing"
         app:column_spacing="8dp"
-        app:row_spacing="8dp"
         app:can_add_items="false">
         <!-- Choices are created in code. -->
     </org.chromium.chrome.browser.autofill_assistant.payment.AssistantChoiceList>
@@ -28,6 +27,8 @@
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
         android:layout_marginTop="8dp"
+        android:layout_marginStart="@dimen/autofill_assistant_bottombar_horizontal_spacing"
+        android:layout_marginEnd="@dimen/autofill_assistant_bottombar_horizontal_spacing"
         android:background="@drawable/autofill_assistant_lightblue_rect_bg"
         android:padding="8dp"
         android:text="@string/autofill_assistant_3rd_party_privacy_notice"

chrome/android/features/autofill_assistant/java/res/values-v17/dimens.xml

@@ -23,8 +23,11 @@
     <dimen name="autofill_assistant_toolbar_vertical_padding">8dp</dimen>
     <dimen name="autofill_assistant_toolbar_swipe_handle_height">4dp</dimen>
     <dimen name="autofill_assistant_choicelist_edit_button_size">20dp</dimen>
-    <dimen name="autofill_assistant_choicelist_add_button_spacing">12dp</dimen>
+    <dimen name="autofill_assistant_minimum_touch_target_size">48dp</dimen>
 
+    <!-- This is less than autofill_assistant_bottombar_horizontal_spacing to allow for larger
+         touch targets at the right side of the PR, while keeping alignment with bottom bar. -->
+    <dimen name="autofill_assistant_payment_request_choice_list_padding_end">12dp</dimen>
     <!-- Amount of padding between a PR section and the next divider (and vice-versa). -->
     <dimen name="autofill_assistant_payment_request_section_padding">12dp</dimen>
     <!-- Amount of padding between PR section title and summary/expanded part. -->

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/payment/AssistantChoiceList.java

@@ -15,6 +15,7 @@ import android.view.ViewGroup;
 import android.widget.CheckBox;
 import android.widget.CompoundButton;
 import android.widget.ImageView;
+import android.widget.LinearLayout;
 import android.widget.RadioButton;
 import android.widget.Space;
 import android.widget.TextView;
@@ -72,11 +73,11 @@ public class AssistantChoiceList extends GridLayout {
     /**
      * |mAddButton| and |mAddButtonLabel| are guaranteed to be non-null if |mCanAddItems| is true.
      */
-    private final ChromeImageView mAddButton;
+    private final View mAddButton;
     private final TextView mAddButtonLabel;
     private final int mRowSpacing;
     private final int mColumnSpacing;
-    private final int mAddButtonSpacing;
+    private final int mMinimumTouchTargetSize;
     private final List<Item> mItems = new ArrayList<>();
     private boolean mAllowMultipleChoices;
     private Runnable mAddButtonListener;
@@ -92,8 +93,8 @@ public class AssistantChoiceList extends GridLayout {
                 : null;
         mRowSpacing = a.getDimensionPixelSize(R.styleable.AssistantChoiceList_row_spacing, 0);
         mColumnSpacing = a.getDimensionPixelSize(R.styleable.AssistantChoiceList_column_spacing, 0);
-        mAddButtonSpacing = context.getResources().getDimensionPixelSize(
-                R.dimen.autofill_assistant_choicelist_add_button_spacing);
+        mMinimumTouchTargetSize = context.getResources().getDimensionPixelSize(
+                R.dimen.autofill_assistant_minimum_touch_target_size);
         a.recycle();
 
         // One column for the radio buttons, one for the content, one for the edit buttons.
@@ -101,17 +102,14 @@ public class AssistantChoiceList extends GridLayout {
 
         if (mCanAddItems) {
             mAddButton = createAddButtonIcon();
-            mAddButtonLabel = createAddButtonLabel(addButtonText);
-
             addViewInternal(mAddButton, -1, createRadioButtonLayoutParams());
+
+            mAddButtonLabel = createAddButtonLabel(addButtonText);
             GridLayout.LayoutParams lp =
                     new GridLayout.LayoutParams(GridLayout.spec(UNDEFINED), GridLayout.spec(1, 2));
-            lp.setGravity(Gravity.FILL_HORIZONTAL | Gravity.CENTER_VERTICAL);
+            lp.setGravity(Gravity.FILL | Gravity.CENTER_VERTICAL);
             lp.width = 0;
             addViewInternal(mAddButtonLabel, -1, lp);
-
-            // Set margin to 0 because list is currently empty.
-            updateAddButtonMargins(0);
         } else {
             mAddButton = null;
             mAddButtonLabel = null;
@@ -167,17 +165,20 @@ public class AssistantChoiceList extends GridLayout {
             @Nullable Runnable itemEditedListener) {
         CompoundButton radioButton =
                 mAllowMultipleChoices ? new CheckBox(getContext()) : new RadioButton(getContext());
+        radioButton.setPadding(0, 0, mColumnSpacing, 0);
+
+        // Ensure minimum touch target size (buttons will auto-scale their height with this view).
+        view.setMinimumHeight(mMinimumTouchTargetSize);
+
         // Insert at end, before the `add' button (if any).
         int viewIndex = mCanAddItems ? indexOfChild(mAddButton) : getChildCount();
         addViewInternal(radioButton, viewIndex++, createRadioButtonLayoutParams());
         addViewInternal(view, viewIndex++, createContentLayoutParams());
 
-        ChromeImageView editButton = null;
-        View spacer = null;
+        View editButton = null;
+        LinearLayout spacer = null;
         if (hasEditButton) {
-            editButton = new ChromeImageView(getContext());
-            editButton.setImageResource(R.drawable.ic_edit_24dp);
-            editButton.setScaleType(ImageView.ScaleType.CENTER_INSIDE);
+            editButton = createEditButton();
             editButton.setOnClickListener(unusedView -> {
                 if (itemEditedListener != null) {
                     itemEditedListener.run();
@@ -185,7 +186,8 @@ public class AssistantChoiceList extends GridLayout {
             });
             addViewInternal(editButton, viewIndex++, createEditButtonLayoutParams());
         } else {
-            spacer = new Space(getContext());
+            spacer = createMinimumTouchSizeContainer();
+            spacer.addView(new Space(getContext()));
             addViewInternal(spacer, viewIndex++, createEditButtonLayoutParams());
         }
 
@@ -199,11 +201,6 @@ public class AssistantChoiceList extends GridLayout {
         radioButton.setOnClickListener(clickListener);
         view.setOnClickListener(clickListener);
         mItems.add(item);
-
-        // Need to adjust button margins after first item was inserted.
-        if (mItems.size() == 1) {
-            updateAddButtonMargins(mAddButtonSpacing);
-        }
     }
 
     /**
@@ -222,7 +219,6 @@ public class AssistantChoiceList extends GridLayout {
             }
         }
         mItems.clear();
-        updateAddButtonMargins(0);
     }
 
     public View getItem(int index) {
@@ -311,15 +307,19 @@ public class AssistantChoiceList extends GridLayout {
         super.addView(view, index, lp);
     }
 
-    private ChromeImageView createAddButtonIcon() {
+    private View createAddButtonIcon() {
         ChromeImageView addButtonIcon = new ChromeImageView(getContext());
         addButtonIcon.setImageResource(R.drawable.ic_autofill_assistant_add_circle_24dp);
-        addButtonIcon.setOnClickListener(unusedView -> {
+        LinearLayout container = new LinearLayout(getContext());
+        container.setGravity(Gravity.CENTER);
+        container.setPadding(0, 0, mColumnSpacing, 0);
+        container.addView(addButtonIcon);
+        container.setOnClickListener(unusedView -> {
             if (mAddButtonListener != null) {
                 mAddButtonListener.run();
             }
         });
-        return addButtonIcon;
+        return container;
     }
 
     private TextView createAddButtonLabel(String addButtonText) {
@@ -332,6 +332,8 @@ public class AssistantChoiceList extends GridLayout {
                 mAddButtonListener.run();
             }
         });
+        addButtonLabel.setMinimumHeight(mMinimumTouchTargetSize);
+        addButtonLabel.setGravity(Gravity.CENTER_VERTICAL);
         return addButtonLabel;
     }
 
@@ -339,7 +341,7 @@ public class AssistantChoiceList extends GridLayout {
         // Set layout params to let content grow to maximum size.
         GridLayout.LayoutParams lp =
                 new GridLayout.LayoutParams(GridLayout.spec(UNDEFINED), GridLayout.spec(1, 1));
-        lp.setGravity(Gravity.FILL_HORIZONTAL | Gravity.CENTER_VERTICAL);
+        lp.setGravity(Gravity.FILL | Gravity.CENTER_VERTICAL);
         lp.width = 0;
         lp.topMargin = mItems.isEmpty() ? 0 : mRowSpacing;
         return lp;
@@ -348,42 +350,38 @@ public class AssistantChoiceList extends GridLayout {
     private GridLayout.LayoutParams createRadioButtonLayoutParams() {
         GridLayout.LayoutParams lp =
                 new GridLayout.LayoutParams(GridLayout.spec(UNDEFINED), GridLayout.spec(0, 1));
-        lp.setGravity(Gravity.CENTER);
-        lp.setMarginEnd(mColumnSpacing);
+        lp.setGravity(Gravity.CENTER | Gravity.FILL);
         lp.topMargin = mItems.isEmpty() ? 0 : mRowSpacing;
         return lp;
     }
 
     private GridLayout.LayoutParams createEditButtonLayoutParams() {
-        int editButtonSize = getContext().getResources().getDimensionPixelSize(
-                R.dimen.autofill_assistant_choicelist_edit_button_size);
         GridLayout.LayoutParams lp =
                 new GridLayout.LayoutParams(GridLayout.spec(UNDEFINED), GridLayout.spec(2, 1));
         lp.setGravity(Gravity.CENTER_VERTICAL | Gravity.FILL_VERTICAL);
         lp.setMarginStart(mColumnSpacing);
-        lp.width = editButtonSize;
-        lp.height = editButtonSize;
         lp.topMargin = mItems.isEmpty() ? 0 : mRowSpacing;
         return lp;
     }
 
-    /**
-     * Adjusts the margins of the 'add' button.
-     *
-     * For empty lists, the margins should be 0. For non-empty lists, the margins should be equal
-     * to |mRowSpacing|.
-     */
-    private void updateAddButtonMargins(int marginTop) {
-        if (!mCanAddItems) {
-            return;
-        }
-
-        LayoutParams lp = (LayoutParams) mAddButton.getLayoutParams();
-        lp.setMargins(lp.leftMargin, marginTop, lp.rightMargin, lp.bottomMargin);
-        mAddButton.setLayoutParams(lp);
+    private View createEditButton() {
+        int editButtonSize = getContext().getResources().getDimensionPixelSize(
+                R.dimen.autofill_assistant_choicelist_edit_button_size);
+        ChromeImageView editButton = new ChromeImageView(getContext());
+        editButton.setImageResource(R.drawable.ic_edit_24dp);
+        editButton.setScaleType(ImageView.ScaleType.CENTER_INSIDE);
+        editButton.setLayoutParams(new ViewGroup.LayoutParams(editButtonSize, editButtonSize));
+
+        LinearLayout editButtonLayout = createMinimumTouchSizeContainer();
+        editButtonLayout.setGravity(Gravity.CENTER);
+        editButtonLayout.addView(editButton);
+        return editButtonLayout;
+    }
 
-        lp = (LayoutParams) mAddButtonLabel.getLayoutParams();
-        lp.setMargins(lp.leftMargin, marginTop, lp.rightMargin, lp.bottomMargin);
-        mAddButtonLabel.setLayoutParams(lp);
+    private LinearLayout createMinimumTouchSizeContainer() {
+        LinearLayout container = new LinearLayout(getContext());
+        container.setMinimumWidth(mMinimumTouchTargetSize);
+        container.setMinimumHeight(mMinimumTouchTargetSize);
+        return container;
     }
 }

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/payment/AssistantPaymentRequestSection.java

@@ -231,7 +231,7 @@ public abstract class AssistantPaymentRequestSection<T extends EditableOption> {
         } else if (mSectionExpander.isExpanded()) {
             // Section is expanded, i.e., the expanded widget is the bottom-most widget.
             mSectionExpander.setTitlePadding(mTopPadding, mTitleToContentPadding);
-            setBottomPadding(mSectionExpander.getExpandedView(), mBottomPadding);
+            // No need to set additional bottom padding, expanded sections have enough already.
         } else {
             // Section is non-empty and collapsed -> collapsed widget is the bottom-most widget.
             mSectionExpander.setTitlePadding(mTopPadding, mTitleToContentPadding);

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/payment/AssistantPaymentRequestTermsSection.java

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.autofill_assistant.payment;
 
 import android.content.Context;
 import android.text.style.StyleSpan;
+import android.view.Gravity;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -38,6 +39,7 @@ public class AssistantPaymentRequestTermsSection {
 
         mTermsAgree = new TextView(context);
         ApiCompatibilityUtils.setTextAppearance(mTermsAgree, R.style.TextAppearance_BlackCaption);
+        mTermsAgree.setGravity(Gravity.CENTER_VERTICAL);
         mTermsList.addItem(mTermsAgree, /*hasEditButton=*/false, selected -> {
             if (selected && mListener != null) {
                 mListener.onResult(AssistantTermsAndConditionsState.ACCEPTED);
@@ -47,6 +49,7 @@ public class AssistantPaymentRequestTermsSection {
         mTermsRequiresReview = new TextView(context);
         ApiCompatibilityUtils.setTextAppearance(
                 mTermsRequiresReview, R.style.TextAppearance_BlackCaption);
+        mTermsRequiresReview.setGravity(Gravity.CENTER_VERTICAL);
         mTermsList.addItem(mTermsRequiresReview, /*hasEditButton=*/false, selected -> {
             if (selected && mListener != null) {
                 mListener.onResult(AssistantTermsAndConditionsState.REQUIRES_REVIEW);