Radio button widget: Use 0dp margin between radio buttons.

RadioButtonWithDescriptionLayout uses a hard coded 8dp margin between
the radio button items. This margin is tweaked to 0dp now according to
UX suggestion.

Bug: 1090155
Change-Id: Ib163df65ffc30d1a57fe1add7229fd835f870bda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310838
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790943}

components/browser_ui/widget/android/java/res/values/dimens.xml

@@ -33,9 +33,6 @@
     <dimen name="radio_button_with_description_and_aux_button_aux_button_length"
         tools:ignore="UnusedResources">61dp</dimen>
 
-    <!-- RadioButton(WithDescription)Layout dimensions -->
-    <dimen name="default_vertical_margin_between_items">8dp</dimen>
-
     <dimen name="list_menu_width">180dp</dimen>
 
     <!-- Custom Menu dimensions -->

components/browser_ui/widget/android/java/src/org/chromium/components/browser_ui/widget/RadioButtonLayout.java

@@ -7,7 +7,6 @@ package org.chromium.components.browser_ui.widget;
 import android.content.Context;
 import android.util.AttributeSet;
 import android.view.LayoutInflater;
-import android.view.View;
 import android.widget.RadioButton;
 import android.widget.RadioGroup;
 
@@ -16,8 +15,7 @@ import androidx.annotation.Nullable;
 import java.util.List;
 
 /**
- * Manages a group of exclusive RadioButtons, automatically inserting a margin in between the rows
- * to prevent them from squishing together.
+ * Manages a group of exclusive RadioButtons.
  *
  * -------------------------------------------------
  * | O | MESSAGE #1                                |
@@ -27,16 +25,12 @@ import java.util.List;
 public final class RadioButtonLayout extends RadioGroup {
     public static final int INVALID_INDEX = -1;
 
-    private final int mMarginBetweenRows;
-
     public RadioButtonLayout(Context context) {
         this(context, null);
     }
 
     public RadioButtonLayout(Context context, AttributeSet attrs) {
         super(context, attrs);
-        mMarginBetweenRows = context.getResources().getDimensionPixelSize(
-                R.dimen.default_vertical_margin_between_items);
     }
 
     /**
@@ -56,8 +50,6 @@ public final class RadioButtonLayout extends RadioGroup {
 
             addView(button, new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT));
         }
-
-        updateMargins();
     }
 
     /**
@@ -76,14 +68,4 @@ public final class RadioButtonLayout extends RadioGroup {
             child.setChecked(i == childIndex);
         }
     }
-
-    /** Sets margins between each of the radio buttons. */
-    private void updateMargins() {
-        int childCount = getChildCount();
-        for (int i = 0; i < childCount; i++) {
-            View child = getChildAt(i);
-            int margin = (i < childCount - 1) ? mMarginBetweenRows : 0;
-            ((MarginLayoutParams) child.getLayoutParams()).bottomMargin = margin;
-        }
-    }
 }

components/browser_ui/widget/android/java/src/org/chromium/components/browser_ui/widget/RadioButtonLayoutTest.java

@@ -58,12 +58,7 @@ public class RadioButtonLayoutTest {
         for (int i = 0; i < layout.getChildCount(); i++) {
             View child = layout.getChildAt(i);
             MarginLayoutParams params = (MarginLayoutParams) child.getLayoutParams();
-
-            if (i < layout.getChildCount() - 1) {
-                Assert.assertNotEquals(0, params.bottomMargin);
-            } else {
-                Assert.assertEquals(0, params.bottomMargin);
-            }
+            Assert.assertEquals(0, params.bottomMargin);
         }
 
         // Add more options.
@@ -77,12 +72,7 @@ public class RadioButtonLayoutTest {
         for (int i = 0; i < layout.getChildCount(); i++) {
             View child = layout.getChildAt(i);
             MarginLayoutParams params = (MarginLayoutParams) child.getLayoutParams();
-
-            if (i < layout.getChildCount() - 1) {
-                Assert.assertNotEquals(0, params.bottomMargin);
-            } else {
-                Assert.assertEquals(0, params.bottomMargin);
-            }
+            Assert.assertEquals(0, params.bottomMargin);
         }
     }
 

components/browser_ui/widget/android/java/src/org/chromium/components/browser_ui/widget/RadioButtonWithDescriptionLayout.java

@@ -19,8 +19,7 @@ import java.util.List;
 
 /**
  * <p>
- * Manages a group of exclusive RadioButtonWithDescriptions, automatically inserting a margin in
- * between the rows to prevent them from squishing together. Has the option to set an accessory view
+ * Manages a group of exclusive RadioButtonWithDescriptions. Has the option to set an accessory view
  * on any given RadioButtonWithDescription. Only one accessory view per layout is supported.
  * <pre>
  * -------------------------------------------------
@@ -53,7 +52,6 @@ import java.util.List;
  */
 public final class RadioButtonWithDescriptionLayout
         extends RadioGroup implements RadioButtonWithDescription.ButtonCheckedStateChangedListener {
-    private final int mMarginBetweenRows;
     private final List<RadioButtonWithDescription> mRadioButtonsWithDescriptions;
     private OnCheckedChangeListener mOnCheckedChangeListener;
 
@@ -63,9 +61,6 @@ public final class RadioButtonWithDescriptionLayout
 
     public RadioButtonWithDescriptionLayout(Context context, AttributeSet attrs) {
         super(context, attrs);
-        mMarginBetweenRows = context.getResources().getDimensionPixelSize(
-                R.dimen.default_vertical_margin_between_items);
-
         mRadioButtonsWithDescriptions = new ArrayList<>();
     }
 
@@ -78,8 +73,6 @@ public final class RadioButtonWithDescriptionLayout
             RadioButtonWithDescription b = (RadioButtonWithDescription) getChildAt(i);
             setupButton(b);
         }
-
-        updateMargins();
     }
 
     /**
@@ -109,8 +102,6 @@ public final class RadioButtonWithDescriptionLayout
             setupButton(b);
             addView(b, new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT));
         }
-
-        updateMargins();
     }
 
     private View removeAttachedAccessoryView(View view) {
@@ -138,18 +129,6 @@ public final class RadioButtonWithDescriptionLayout
         addView(accessoryView, attachmentPointIndex + 1);
     }
 
-    /** Sets margins between each of the radio buttons. */
-    private void updateMargins() {
-        int childCount = getChildCount();
-        for (int i = 0; i < childCount - 1; i++) {
-            View child = getChildAt(i);
-            MarginLayoutParams params = (MarginLayoutParams) child.getLayoutParams();
-            params.bottomMargin = mMarginBetweenRows;
-        }
-        // LayoutParam changes only take effect after the next layout pass.
-        requestLayout();
-    }
-
     private void setupButton(RadioButtonWithDescription radioButton) {
         radioButton.setOnCheckedChangeListener(this);
         // Give the button a unique id to allow for calling onCheckedChanged (see

components/browser_ui/widget/android/java/src/org/chromium/components/browser_ui/widget/RadioButtonWithDescriptionLayoutTest.java

@@ -34,8 +34,6 @@ import java.util.List;
  */
 @RunWith(BaseJUnit4ClassRunner.class)
 public class RadioButtonWithDescriptionLayoutTest {
-    private static final String NON_ZERO_MARGIN_ASSERT_MESSAGE =
-            "First N-1 items should have a non-zero margin";
     private static final String ZERO_MARGIN_ASSERT_MESSAGE =
             "The last item should have a zero margin";
     private static final String PRIMARY_MATCH_ASSERT_MESSAGE =
@@ -80,12 +78,7 @@ public class RadioButtonWithDescriptionLayoutTest {
         for (int i = 0; i < layout.getChildCount(); i++) {
             View child = layout.getChildAt(i);
             MarginLayoutParams params = (MarginLayoutParams) child.getLayoutParams();
-
-            if (i < layout.getChildCount() - 1) {
-                Assert.assertNotEquals(NON_ZERO_MARGIN_ASSERT_MESSAGE, 0, params.bottomMargin);
-            } else {
-                Assert.assertEquals(ZERO_MARGIN_ASSERT_MESSAGE, 0, params.bottomMargin);
-            }
+            Assert.assertEquals(ZERO_MARGIN_ASSERT_MESSAGE, 0, params.bottomMargin);
         }
 
         // Add more buttons.
@@ -100,12 +93,7 @@ public class RadioButtonWithDescriptionLayoutTest {
         for (int i = 0; i < layout.getChildCount(); i++) {
             View child = layout.getChildAt(i);
             MarginLayoutParams params = (MarginLayoutParams) child.getLayoutParams();
-
-            if (i < layout.getChildCount() - 1) {
-                Assert.assertNotEquals(NON_ZERO_MARGIN_ASSERT_MESSAGE, 0, params.bottomMargin);
-            } else {
-                Assert.assertEquals(ZERO_MARGIN_ASSERT_MESSAGE, 0, params.bottomMargin);
-            }
+            Assert.assertEquals(ZERO_MARGIN_ASSERT_MESSAGE, 0, params.bottomMargin);
         }
     }