From 20d37cb262991a8d7c4e7a5153abc5aeff915dc3 Mon Sep 17 00:00:00 2001
From: Adam Barth <abarth@chromium.org>
Date: Wed, 19 Aug 2015 18:01:02 -0700
Subject: [PATCH] Fix padding on infinite scrolling list

Rather than using a Padding widget to provide padding along the scrolling axis,
we now just figure the padding into where we draw the items. This patch fixes
an issue where we would remove the first topmost item in a scrollable list too
early because we thought it was already off screen.

Fixes #697
---
 sky/packages/sky/lib/widgets/scrollable.dart | 46 ++++++++++++++------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/sky/packages/sky/lib/widgets/scrollable.dart b/sky/packages/sky/lib/widgets/scrollable.dart
index a6d5d72b7013d..c8c5a3f45d167 100644
--- a/sky/packages/sky/lib/widgets/scrollable.dart
+++ b/sky/packages/sky/lib/widgets/scrollable.dart
@@ -384,14 +384,30 @@ abstract class FixedHeightScrollable extends Scrollable {
     });
   }
 
+  double get _leadingPadding {
+    if (scrollDirection == ScrollDirection.vertical)
+      return padding.top;
+    return padding.left;
+  }
+
+  double get _trailingPadding {
+    if (scrollDirection == ScrollDirection.vertical)
+      return padding.bottom;
+    return padding.right;
+  }
+
+  EdgeDims get _crossAxisPadding {
+    if (padding == null)
+      return null;
+    if (scrollDirection == ScrollDirection.vertical)
+      return new EdgeDims.only(left: padding.left, right: padding.right);
+    return new EdgeDims.only(top: padding.top, bottom: padding.bottom);
+  }
+
   void _updateContentsExtent() {
     double contentsExtent = itemExtent * itemCount;
-    if (padding != null) {
-      if (scrollDirection == ScrollDirection.vertical)
-        contentsExtent += padding.top + padding.bottom;
-      else
-        contentsExtent += padding.left + padding.right;
-    }
+    if (padding != null)
+      contentsExtent += _leadingPadding + _trailingPadding;
     scrollBehavior.contentsSize = contentsExtent;
   }
 
@@ -413,25 +429,29 @@ abstract class FixedHeightScrollable extends Scrollable {
       _updateScrollOffset();
     }
 
+    double paddedScrollOffset = scrollOffset;
+    if (padding != null)
+      paddedScrollOffset -= _leadingPadding;
+
     int itemShowIndex = 0;
     int itemShowCount = 0;
     Offset viewportOffset = Offset.zero;
     if (_containerExtent != null && _containerExtent > 0.0) {
-      if (scrollOffset < 0.0) {
-        double visibleHeight = _containerExtent + scrollOffset;
+      if (paddedScrollOffset < 0.0) {
+        double visibleHeight = _containerExtent + paddedScrollOffset;
         itemShowCount = (visibleHeight / itemExtent).round() + 1;
-        viewportOffset = _toOffset(scrollOffset);
+        viewportOffset = _toOffset(paddedScrollOffset);
       } else {
         itemShowCount = (_containerExtent / itemExtent).ceil();
-        double alignmentDelta = -scrollOffset % itemExtent;
+        double alignmentDelta = -paddedScrollOffset % itemExtent;
         double drawStart;
         if (alignmentDelta != 0.0) {
           alignmentDelta -= itemExtent;
           itemShowCount += 1;
-          drawStart = scrollOffset + alignmentDelta;
+          drawStart = paddedScrollOffset + alignmentDelta;
           viewportOffset = _toOffset(-alignmentDelta);
         } else {
-          drawStart = scrollOffset;
+          drawStart = paddedScrollOffset;
         }
         itemShowIndex = math.max(0, (drawStart / itemExtent).floor());
       }
@@ -453,7 +473,7 @@ abstract class FixedHeightScrollable extends Scrollable {
         scrollDirection: scrollDirection,
         scrollOffset: viewportOffset,
         child: new Container(
-          padding: padding,
+          padding: _crossAxisPadding,
           child: new Block(items, direction: blockDirection)
         )
       )