[FeedV2]: Remove negative margins in header and add animation for border transition.

Negative margins are generally discouraged. We use a TouchDelegate instead to
increase the size of the touchable area surrounding the icon.

We also add a padding-based animation to transition between the borderless "on" state and the bordered "off" state

Bug: 1123996, 1135402, b/167409361
Change-Id: Ie51d4af5ed014d69d07981d486addd0a4643b76a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2451399
Commit-Queue: Cathy Li <chili@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818514}

chrome/android/java/res/layout/new_tab_page_feed_v2_expandable_header.xml

@@ -14,7 +14,7 @@
     android:orientation="horizontal"
     android:gravity="center_vertical"
     android:layoutDirection="locale"
-    app:showHairlineWhenDisabled="false">
+    app:animatePaddingWhenDisabled="true">
 
     <!-- Note: Setting textDirection to "locale" makes sure that a non-translated English word in
          the title text view is aligned based on the device locale and not the text content.
@@ -27,18 +27,20 @@
         android:textAppearance="@style/TextAppearance.TextAccentMediumThick.Secondary"
         android:textDirection="locale" />
 
+    <!-- Note ignore RtlHardcoded since this is an image, and the paddingLeft is for
+         spacing the image away from the header title above.
+    -->
     <org.chromium.components.browser_ui.widget.listmenu.ListMenuButton
         android:id="@+id/header_menu"
-        android:layout_width="@dimen/snippets_article_header_menu_size"
+        android:layout_width="@dimen/feed_v2_header_menu_width"
         android:layout_height="@dimen/snippets_article_header_menu_size"
         android:scaleType="fitXY"
-        android:padding="15dp"
-        android:layout_marginTop="@dimen/feed_v2_header_menu_vertical_inset"
-        android:layout_marginBottom="@dimen/feed_v2_header_menu_vertical_inset"
-        android:layout_marginEnd="@dimen/feed_v2_header_menu_end_inset"
+        android:paddingVertical="15dp"
+        android:paddingStart="15dp"
         android:background="@null"
         android:src="@drawable/ic_settings_gear_24dp"
         android:contentDescription="@string/accessibility_ntp_feed_menu_button"
         app:menuMaxWidth="@dimen/feed_header_menu_max_width"
-        app:tint="@null" />
+        app:tint="@null"
+        tools:ignore="RtlHardcoded"/>
 </org.chromium.chrome.browser.ntp.snippets.SectionHeaderView>

chrome/android/java/res/values/attrs.xml

@@ -22,6 +22,6 @@
     </declare-styleable>
 
     <declare-styleable name="SectionHeaderView">
-        <attr name="showHairlineWhenDisabled" format="boolean"/>
+        <attr name="animatePaddingWhenDisabled" format="boolean"/>
     </declare-styleable>
 </resources>

chrome/android/java/res/values/dimens.xml

@@ -358,9 +358,10 @@
     <dimen name="ntp_wide_card_lateral_margins_v2">36dp</dimen>
     <dimen name="snippets_article_header_height">40dp</dimen>
     <dimen name="snippets_article_header_menu_size">48dp</dimen>
-    <dimen name="feed_v2_header_menu_vertical_inset">-4dp</dimen>
+    <dimen name="feed_v2_header_menu_width">33dp</dimen>
+    <dimen name="feed_v2_header_menu_touch_padding">4dp</dimen>
+    <dimen name="feed_v2_header_menu_disabled_padding">11dp</dimen>
     <!-- This should match |ntp_header_lateral_margins_v2|. -->
-    <dimen name="feed_v2_header_menu_end_inset">-16dp</dimen>
     <dimen name="ntp_header_lateral_margins_v2">16dp</dimen>
     <!-- This is in sp because we want the icon to scale with the TextView it sits alongside. -->
     <dimen name="content_suggestions_card_modern_margin">12dp</dimen>

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SectionHeaderView.java

@@ -4,10 +4,14 @@
 
 package org.chromium.chrome.browser.ntp.snippets;
 
+import android.animation.Animator;
+import android.animation.AnimatorListenerAdapter;
+import android.animation.ValueAnimator;
 import android.content.Context;
 import android.content.res.TypedArray;
 import android.graphics.Rect;
 import android.util.AttributeSet;
+import android.view.TouchDelegate;
 import android.view.View;
 import android.widget.LinearLayout;
 import android.widget.TextView;
@@ -34,6 +38,7 @@ import org.chromium.ui.widget.ViewRectProvider;
  */
 public class SectionHeaderView extends LinearLayout implements View.OnClickListener {
     private static final int IPH_TIMEOUT_MS = 10000;
+    private static final int ANIMATION_DURATION_MS = 200;
 
     // Views in the header layout that are set during inflate.
     private TextView mTitleView;
@@ -45,7 +50,7 @@ public class SectionHeaderView extends LinearLayout implements View.OnClickListe
     private SectionHeader mHeader;
 
     private boolean mHasMenu;
-    private boolean mHairlineWhenDisabled = true;
+    private boolean mAnimatePaddingWhenDisabled;
 
     public SectionHeaderView(Context context, @Nullable AttributeSet attrs) {
         super(context, attrs);
@@ -53,8 +58,8 @@ public class SectionHeaderView extends LinearLayout implements View.OnClickListe
                 attrs, R.styleable.SectionHeaderView, 0, 0);
 
         try {
-            mHairlineWhenDisabled = attrArray.getBoolean(
-                    R.styleable.SectionHeaderView_showHairlineWhenDisabled, true);
+            mAnimatePaddingWhenDisabled = attrArray.getBoolean(
+                    R.styleable.SectionHeaderView_animatePaddingWhenDisabled, false);
         } finally {
             attrArray.recycle();
         }
@@ -74,6 +79,25 @@ public class SectionHeaderView extends LinearLayout implements View.OnClickListe
 
         if (mHasMenu) {
             mMenuView.setOnClickListener((View v) -> { displayMenu(); });
+            int touchPadding;
+            // If we are animating padding, add additional touch area around the menu.
+            if (mAnimatePaddingWhenDisabled) {
+                touchPadding = getResources().getDimensionPixelSize(
+                        R.dimen.feed_v2_header_menu_touch_padding);
+            } else {
+                touchPadding = 0;
+            }
+            post(() -> {
+                Rect rect = new Rect();
+                mMenuView.getHitRect(rect);
+
+                rect.top -= touchPadding;
+                rect.bottom += touchPadding;
+                rect.left -= touchPadding;
+                rect.right += touchPadding;
+
+                setTouchDelegate(new TouchDelegate(rect, mMenuView));
+            });
         }
     }
 
@@ -114,9 +138,39 @@ public class SectionHeaderView extends LinearLayout implements View.OnClickListe
                 mStatusView.setText(
                         mHeader.isExpanded() ? R.string.hide_content : R.string.show_content);
             }
-            setBackgroundResource(mHeader.isExpanded() || !mHairlineWhenDisabled
-                            ? 0
-                            : R.drawable.hairline_border_card_background);
+            if (mAnimatePaddingWhenDisabled) {
+                int finalHorizontalPadding = 0;
+                boolean isClosingHeader = !mHeader.isExpanded();
+                if (isClosingHeader) {
+                    // If closing header, add additional padding.
+                    finalHorizontalPadding = getResources().getDimensionPixelSize(
+                            R.dimen.feed_v2_header_menu_disabled_padding);
+                } else {
+                    // Otherwise, remove the background now.
+                    setBackgroundResource(0);
+                }
+                ValueAnimator animator =
+                        ValueAnimator.ofInt(getPaddingLeft(), finalHorizontalPadding);
+                animator.addUpdateListener((ValueAnimator animation) -> {
+                    int horizontalPadding = (Integer) animation.getAnimatedValue();
+                    setPadding(/*left*/ horizontalPadding, getPaddingTop(),
+                            /*right*/ horizontalPadding, getPaddingBottom());
+                });
+                animator.addListener(new AnimatorListenerAdapter() {
+                    @Override
+                    public void onAnimationEnd(Animator animation) {
+                        // If we closed the header, add the hairline after animation.
+                        if (isClosingHeader) {
+                            setBackgroundResource(R.drawable.hairline_border_card_background);
+                        }
+                    }
+                });
+                animator.setDuration(ANIMATION_DURATION_MS);
+                animator.start();
+            } else {
+                setBackgroundResource(
+                        mHeader.isExpanded() ? 0 : R.drawable.hairline_border_card_background);
+            }
         }
     }