NTP, change the card elevation from setElevation to a 9 patch file

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
index faa44c8979d9063d6f49fe64eca64bb86e526c1e..86eed0dda7dbd74610deadfd3be7f9c9a5f9504a 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
@@ -12,7 +12,6 @@ import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.view.animation.Interpolator;
 
-import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.util.MathUtils;
 
@@ -45,28 +44,32 @@ public class CardViewHolder extends NewTabPageViewHolder {
      * and is not peeking anymore.
      */
     private int mPeekPadding;
+    private final int mCards9PatchAdjustment;
+
+    private final NewTabPageRecyclerView mRecyclerView;
 
     /**
      * @param layoutId resource id of the layout to inflate and to use as card.
-     * @param parent ViewGroup that will contain the newly created view.
+     * @param recyclerView ViewGroup that will contain the newly created view.
      */
-    public CardViewHolder(int layoutId, final NewTabPageRecyclerView parent) {
-        super(inflateView(layoutId, parent));
+    public CardViewHolder(int layoutId, final NewTabPageRecyclerView recyclerView) {
+        super(inflateView(layoutId, recyclerView));
+
+        mCards9PatchAdjustment = recyclerView.getResources().getDimensionPixelSize(
+                R.dimen.snippets_card_adjust_9_patch);
 
-        mMaxPeekPadding = parent.getResources().getDimensionPixelSize(
+        mMaxPeekPadding = recyclerView.getResources().getDimensionPixelSize(
                 R.dimen.snippets_padding_and_peeking_card_height);
         mPeekPadding = mMaxPeekPadding;
 
-        ApiCompatibilityUtils.setElevation(
-                itemView, parent.getContext().getResources().getDimensionPixelSize(
-                                  R.dimen.snippets_card_elevation));
+        mRecyclerView = recyclerView;
 
         itemView.setOnClickListener(new OnClickListener() {
 
             @Override
             public void onClick(View v) {
                 if (isPeeking()) {
-                    parent.showCardsFrom(mPeekPadding);
+                    recyclerView.showCardsFrom(mPeekPadding);
                 } else {
                     onCardTapped();
                 }
@@ -89,6 +92,33 @@ public class CardViewHolder extends NewTabPageViewHolder {
         itemView.setAlpha(1f);
     }
 
+    @Override
+    public void updateLayoutParams() {
+        RecyclerView.LayoutParams params = getParams();
+
+        int childPos = mRecyclerView.getChildAdapterPosition(itemView);

[Bernhard Bauer, 2016/06/29 11:22:27]

You can get this directly via getAdapterPosition().

[mcwilliams, 2016/06/29 14:15:55]

Done.

+
+        // We have used 9 patch files for the card elevation instead of android setElevation
+        // due to tab switching and rendering of server shadows. Each card has the full elevation

[Bernhard Bauer, 2016/06/29 11:22:27]

https://codereview.chromium.org/2088793004/diff/40001/chrome/android/java/src...
still applies :)

[mcwilliams, 2016/06/29 14:15:55]

This was missing a commit - updated

+        // effect so we will remove bottom margin to overlay and hide the bottom shadow of the
+        // previous card to give the effect of a divider instead of a shadow.
+        if (mRecyclerView.getAdapter().getItemViewType(childPos + 1)
+                == NewTabPageListItem.VIEW_TYPE_SNIPPET) {
+            params.bottomMargin = -mCards9PatchAdjustment;
+        } else {
+            params.bottomMargin = 0;
+        }
+
+        // Due to the card background being a 9 patch file - the card border shadow will be part of
+        // the card width and height. This mCards9PatchAdjustment value will be used to adjust the
+        // margins so the card width is the actual width not including the elevation shadow so we
+        // can have full bleed.
+        if (!isPeeking()) {

[Bernhard Bauer, 2016/06/29 11:22:27]

Wait, doesn't this evaluate to `getParams().leftMargin ==
-mCards9PatchAdjustment`, which would make this a no-op?

[mcwilliams, 2016/06/29 14:15:55]

Done.

+            params.leftMargin = -mCards9PatchAdjustment;
+            params.rightMargin = -mCards9PatchAdjustment;
+        }
+    }
+
     /**
      * Change the width, padding and child opacity of the card to give a smooth transition as the
      * user scrolls.
@@ -105,7 +135,7 @@ public class CardViewHolder extends NewTabPageViewHolder {
 
     /** Returns whether the card is currently peeking. */
     public boolean isPeeking() {
-        return mPeekPadding < mMaxPeekPadding;
+        return getParams().leftMargin != -mCards9PatchAdjustment;

[Bernhard Bauer, 2016/06/29 11:22:27]

OK, finding out the peeking state from the layout parameters is very awkward. I
would propose we change |mPeekPadding| to a value between 0 and 1 that indicates
to what extent the card is peeking, and we derive everything else from that.
That should also fix the bug below :)

[mcwilliams, 2016/06/29 14:15:55]

Added a TODO as discussed

     }
 
     @Override
@@ -122,13 +152,17 @@ public class CardViewHolder extends NewTabPageViewHolder {
     protected void onCardTapped() {}
 
     private void setPeekingStateForPadding(int padding) {
-        mPeekPadding = padding;
+        // Due to the card background being a 9 patch file - the card border shadow will be part of
+        // the card width and height. This mCards9PatchAdjustment value will be used to adjust the
+        // padding so the card width is the actual width not including the elevation shadow so we
+        // can have full bleed.

[Bernhard Bauer, 2016/06/29 11:22:27]

I think I would move this comment to Javadoc for |mCards9PatchAdjustment|
instead of duplicating it.

[mcwilliams, 2016/06/29 14:15:55]

Done.

+        mPeekPadding = padding + mCards9PatchAdjustment;
 
         // Modify the padding so as the margin increases, the padding decreases, keeping the card's
         // contents in the same position. The top and bottom remain the same.
         itemView.setPadding(mPeekPadding, mMaxPeekPadding, mPeekPadding, mMaxPeekPadding);
 
-        RecyclerView.LayoutParams params = (RecyclerView.LayoutParams) itemView.getLayoutParams();
+        RecyclerView.LayoutParams params = getParams();
         params.leftMargin = mMaxPeekPadding - mPeekPadding;
         params.rightMargin = mMaxPeekPadding - mPeekPadding;
 
@@ -143,4 +177,8 @@ public class CardViewHolder extends NewTabPageViewHolder {
     private static View inflateView(int resourceId, ViewGroup parent) {
         return LayoutInflater.from(parent.getContext()).inflate(resourceId, parent, false);
     }
+
+    private RecyclerView.LayoutParams getParams() {
+        return (RecyclerView.LayoutParams) itemView.getLayoutParams();
+    }
 }