NTP, peeking card and heading hiding constraints

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 c5a2ef4e8e4ba590433cc31f6878ceba029fb3b4..1204db61baf82d724ff78332e6aa1254ec07214a 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
@@ -132,6 +132,16 @@ public class CardViewHolder extends NewTabPageViewHolder {
         return mPeekPadding < mMaxPeekPadding;
     }
 
+    /**
+     * Only allow the card to peek if the user has not scrolled on the page beyond the

[Bernhard Bauer, 2016/07/18 14:19:17]

This seems more like an implementation comment, not a comment documenting the
contract.

[mcwilliams, 2016/07/18 15:11:45]

Done.

+     * |mMaxPeekPadding| height. There will only ever be one peeking card on the page and only when
+     * there is enough space to show the peeking card on the first screen at the bottom.
+     * @return

[Michael van Ouwerkerk, 2016/07/15 15:05:28]

nit: either delete or fully document this @return

[mcwilliams, 2016/07/18 15:11:45]

eclipse pulled a sneaky and added :) removed

+     */
+    public boolean allowCardToPeek() {

[Michael van Ouwerkerk, 2016/07/15 15:05:28]

nit: "isCardAllowedToPeek" seems more appropriate for this method

[PEConn, 2016/07/15 15:08:23]

(I think 'isCardPeekAllowed' is slightly nicer)

[Bernhard Bauer, 2016/07/18 14:19:17]

To add my opinion about the prefered color of the bikeshed: Why even include
"card" here, if the whole class is specific to cards? Why not just canPeek()?

[mcwilliams, 2016/07/18 15:11:45]

After much deliberation I have gone with canPeek :) Thanks all

+        return mRecyclerView.computeVerticalScrollOffset() <= itemView.getHeight();

[PEConn, 2016/07/15 14:03:26]

Can't we use functions from the RecyclerView's LayoutManager
(http://stackoverflow.com/a/25053500) ?

[mcwilliams, 2016/07/18 15:11:45]

Can you be a little more specific here please

+    }
+
     @Override
     public void updateViewStateForDismiss(float dX) {
         float input = Math.abs(dX) / itemView.getMeasuredWidth();
@@ -148,7 +158,7 @@ public class CardViewHolder extends NewTabPageViewHolder {
     private void setPeekingStateForPadding(int padding) {
         // TODO(mcwilliams): Change the 'padding' value to a percentage of what the transition
         // should be which can be used for alpha and bleed.
-        mPeekPadding = padding;
+        mPeekPadding = allowCardToPeek() ? padding : mMaxPeekPadding;
 
         // 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.