From ab13ea240d5d19c08ce30919d390e39a4036beeb Mon Sep 17 00:00:00 2001
From: Adam Barth <abarth@chromium.org>
Date: Fri, 28 Aug 2015 15:00:39 -0700
Subject: [PATCH] onScrollStart shouldn't have an offset

Instead, we pump a onScrollUpdate with the offset if there is an offset.
---
 sky/packages/sky/lib/gestures/scroll.dart    | 16 +++++++++------
 sky/packages/sky/lib/widgets/scrollable.dart |  5 ++---
 sky/unit/test/gestures/scroll_test.dart      | 21 ++++++++++----------
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/sky/packages/sky/lib/gestures/scroll.dart b/sky/packages/sky/lib/gestures/scroll.dart
index 32d586d5ad5..7be12201ad7 100644
--- a/sky/packages/sky/lib/gestures/scroll.dart
+++ b/sky/packages/sky/lib/gestures/scroll.dart
@@ -14,7 +14,7 @@ enum ScrollState {
   accepted
 }
 
-typedef void GestureScrollStartCallback(sky.Offset scrollDelta);
+typedef void GestureScrollStartCallback();
 typedef void GestureScrollUpdateCallback(sky.Offset scrollDelta);
 typedef void GestureScrollEndCallback();
 
@@ -47,7 +47,8 @@ class ScrollGestureRecognizer extends GestureRecognizer {
     if (event.type == 'pointermove') {
       sky.Offset offset = _getScrollOffset(event);
       if (state == ScrollState.accepted) {
-        onScrollUpdate(offset);
+        if (onScrollUpdate != null)
+          onScrollUpdate(offset);
       } else {
         pendingScrollOffset += offset;
         if (pendingScrollOffset.distance > kTouchSlop)
@@ -62,14 +63,17 @@ class ScrollGestureRecognizer extends GestureRecognizer {
       state = ScrollState.accepted;
       sky.Offset offset = pendingScrollOffset;
       pendingScrollOffset = null;
-      onScrollStart(offset);
+      if (onScrollStart != null)
+        onScrollStart();
+      if (offset != sky.Offset.zero && onScrollUpdate != null)
+        onScrollUpdate(offset);
     }
   }
 
   void didStopTrackingLastPointer() {
-     bool wasAccepted = (state == ScrollState.accepted);
-     state = ScrollState.ready;
-     if (wasAccepted)
+    bool wasAccepted = (state == ScrollState.accepted);
+    state = ScrollState.ready;
+    if (wasAccepted && onScrollEnd != null)
       onScrollEnd();
   }
 
diff --git a/sky/packages/sky/lib/widgets/scrollable.dart b/sky/packages/sky/lib/widgets/scrollable.dart
index 3c39c083307..2f4e2a76918 100644
--- a/sky/packages/sky/lib/widgets/scrollable.dart
+++ b/sky/packages/sky/lib/widgets/scrollable.dart
@@ -84,8 +84,7 @@ abstract class Scrollable extends StatefulComponent {
 
   Widget build() {
     return new GestureDetector(
-      onScrollStart: _handleScrollOffset,
-      onScrollUpdate: _handleScrollOffset,
+      onScrollUpdate: _handleScrollUpdate,
       onScrollEnd: _maybeSettleScrollOffset,
       child: new Listener(
         child: buildContent(),
@@ -173,7 +172,7 @@ abstract class Scrollable extends StatefulComponent {
     return EventDisposition.processed;
   }
 
-  void _handleScrollOffset(Offset offset) {
+  void _handleScrollUpdate(Offset offset) {
     scrollBy(scrollDirection == ScrollDirection.horizontal ? offset.dx : offset.dy);
   }
 
diff --git a/sky/unit/test/gestures/scroll_test.dart b/sky/unit/test/gestures/scroll_test.dart
index 2806d30302e..f56358297f7 100644
--- a/sky/unit/test/gestures/scroll_test.dart
+++ b/sky/unit/test/gestures/scroll_test.dart
@@ -43,9 +43,9 @@ void main() {
     PointerRouter router = new PointerRouter();
     ScrollGestureRecognizer scroll = new ScrollGestureRecognizer(router: router);
 
-    sky.Offset startOffset;
-    scroll.onScrollStart = (sky.Offset offset) {
-      startOffset = offset;
+    bool didStartScroll = false;
+    scroll.onScrollStart = () {
+      didStartScroll = true;
     };
 
     sky.Offset updateOffset;
@@ -59,29 +59,30 @@ void main() {
     };
 
     scroll.addPointer(down);
-    expect(startOffset, isNull);
+    expect(didStartScroll, isFalse);
     expect(updateOffset, isNull);
     expect(didEndScroll, isFalse);
 
     router.handleEvent(down, null);
-    expect(startOffset, isNull);
+    expect(didStartScroll, isFalse);
     expect(updateOffset, isNull);
     expect(didEndScroll, isFalse);
 
     router.handleEvent(move1, null);
-    expect(startOffset, new sky.Offset(10.0, -10.0));
-    startOffset = null;
-    expect(updateOffset, isNull);
+    expect(didStartScroll, isTrue);
+    didStartScroll = false;
+    expect(updateOffset, new sky.Offset(10.0, -10.0));
+    updateOffset = null;
     expect(didEndScroll, isFalse);
 
     router.handleEvent(move2, null);
-    expect(startOffset, isNull);
+    expect(didStartScroll, isFalse);
     expect(updateOffset, new sky.Offset(0.0, -5.0));
     updateOffset = null;
     expect(didEndScroll, isFalse);
 
     router.handleEvent(up, null);
-    expect(startOffset, isNull);
+    expect(didStartScroll, isFalse);
     expect(updateOffset, isNull);
     expect(didEndScroll, isTrue);
     didEndScroll = false;