Merge pull request #1012 from HansMuller/improve-items-wrap

Restore PageableList itemsWrap:true

Makes PageableList with itemsWrap:true work again.

Plumbed the itemsWrap parameter through to HomogenousViewport.

Fixes issue #877.

packages/flutter/lib/widgets/homogeneous_viewport.dart

@@ -16,6 +16,7 @@ class HomogeneousViewport extends RenderObjectWrapper {
   HomogeneousViewport({
     Key key,
     this.builder,
+    this.itemsWrap: false,
     this.itemExtent, // required
     this.itemCount, // optional, but you cannot shrink-wrap this class or otherwise use its intrinsic dimensions if you don't specify it
     this.direction: ScrollDirection.vertical,
@@ -25,6 +26,7 @@ class HomogeneousViewport extends RenderObjectWrapper {
   }
 
   ListBuilder builder;
+  bool itemsWrap;
   double itemExtent;
   int itemCount;
   ScrollDirection direction;
@@ -115,16 +117,16 @@ class HomogeneousViewport extends RenderObjectWrapper {
     try {
       double mainAxisExtent = direction == ScrollDirection.vertical ? constraints.maxHeight : constraints.maxWidth;
       double offset;
-      if (startOffset <= 0.0) {
+      if (startOffset <= 0.0 && !itemsWrap) {
         _layoutFirstIndex = 0;
         offset = -startOffset;
       } else {
-        _layoutFirstIndex = startOffset ~/ itemExtent;
+        _layoutFirstIndex = (startOffset / itemExtent).floor();
         offset = -(startOffset % itemExtent);
       }
       if (mainAxisExtent < double.INFINITY) {
         _layoutItemCount = ((mainAxisExtent - offset) / itemExtent).ceil();
-        if (itemCount != null)
+        if (itemCount != null && !itemsWrap)
           _layoutItemCount = math.min(_layoutItemCount, itemCount - _layoutFirstIndex);
       } else {
         assert(() {

packages/flutter/lib/widgets/scrollable.dart

@@ -348,6 +348,7 @@ abstract class ScrollableWidgetList extends Scrollable {
     Key key,
     double initialScrollOffset,
     ScrollDirection scrollDirection: ScrollDirection.vertical,
+    this.itemsWrap: false,
     this.itemExtent,
     this.padding
   }) : super(key: key, initialScrollOffset: initialScrollOffset, scrollDirection: scrollDirection) {
@@ -355,6 +356,7 @@ abstract class ScrollableWidgetList extends Scrollable {
   }
 
   EdgeDims padding;
+  bool itemsWrap;
   double itemExtent;
   Size containerSize = Size.zero;
 
@@ -441,6 +443,7 @@ abstract class ScrollableWidgetList extends Scrollable {
         padding: _crossAxisPadding,
         child: new HomogeneousViewport(
           builder: _buildItems,
+          itemsWrap: itemsWrap,
           itemExtent: itemExtent,
           itemCount: itemCount,
           direction: scrollDirection,
@@ -472,19 +475,19 @@ class ScrollableList<T> extends ScrollableWidgetList {
     ScrollDirection scrollDirection: ScrollDirection.vertical,
     this.items,
     this.itemBuilder,
-    this.itemsWrap: false,
+    itemsWrap: false,
     double itemExtent,
     EdgeDims padding
   }) : super(
     key: key,
     initialScrollOffset: initialScrollOffset,
     scrollDirection: scrollDirection,
+    itemsWrap: itemsWrap,
     itemExtent: itemExtent,
     padding: padding);
 
   List<T> items;
   ItemBuilder<T> itemBuilder;
-  bool itemsWrap;
 
   void syncConstructorArguments(ScrollableList<T> source) {
     items = source.items;
@@ -568,7 +571,7 @@ class PageableList<T> extends ScrollableList<T> {
     return EventDisposition.processed;
   }
 
-  int get currentPage => (scrollOffset / itemExtent).floor();
+  int get currentPage => (scrollOffset / itemExtent).floor() % itemCount;
 
   void _notifyPageChanged(_) {
     if (pageChanged != null)

packages/unit/test/widget/pageable_list_test.dart

@@ -4,49 +4,89 @@ import 'package:test/test.dart';
 
 import 'widget_tester.dart';
 
+const Size pageSize = const Size(800.0, 600.0);
+const List<int> pages = const <int>[0, 1, 2, 3, 4, 5];
+int currentPage = null;
+
+Widget buildPage(int page) {
+  return new Container(
+    key: new ValueKey<int>(page),
+    width: pageSize.width,
+    height: pageSize.height,
+    child: new Text(page.toString())
+  );
+}
+
+Widget buildFrame({ bool itemsWrap: false }) {
+  // The test framework forces the frame (and so the PageableList)
+  // to be 800x600. The pageSize constant reflects as much.
+  return new PageableList<int>(
+    items: pages,
+    itemBuilder: buildPage,
+    itemsWrap: itemsWrap,
+    itemExtent: pageSize.width,
+    scrollDirection: ScrollDirection.horizontal,
+    pageChanged: (int page) { currentPage = page; }
+  );
+}
+
+void page(WidgetTester tester, Offset offset) {
+  String itemText = currentPage != null ? currentPage.toString() : '0';
+  new FakeAsync().run((async) {
+    tester.scroll(tester.findText(itemText), offset);
+    // One frame to start the animation, a second to complete it.
+    tester.pumpFrame(buildFrame);
+    tester.pumpFrame(buildFrame, 1000.0);
+    async.elapse(new Duration(seconds: 1));
+  });
+}
+
+void pageLeft(WidgetTester tester) {
+  page(tester, new Offset(-pageSize.width, 0.0));
+}
+
+void pageRight(WidgetTester tester) {
+  page(tester, new Offset(pageSize.width, 0.0));
+}
+
 void main() {
-  test('Scrolling changes page', () {
+  // PageableList with itemsWrap: false
+
+  test('Scroll left from page 0 to page 1', () {
+    WidgetTester tester = new WidgetTester();
+    currentPage = null;
+    tester.pumpFrame(buildFrame);
+    expect(currentPage, isNull);
+    pageLeft(tester);
+    expect(currentPage, equals(1));
+  });
+
+  test('Underscroll (scroll right), return to page 0', () {
     WidgetTester tester = new WidgetTester();
+    currentPage = null;
+    tester.pumpFrame(buildFrame);
+    expect(currentPage, isNull);
+    pageRight(tester);
+    expect(currentPage, equals(0));
+  });
+
+  // PageableList with itemsWrap: true
 
-    List<int> pages = [0, 1, 2, 3, 4, 5];
-    Size pageSize = new Size(200.0, 200.0);
-    int currentPage;
-
-    Widget buildPage(int page) {
-      return new Container(
-        key: new ValueKey<int>(page),
-        width: pageSize.width,
-        height: pageSize.height,
-        child: new Text(page.toString())
-      );
-    }
-
-    Widget builder() {
-      return new Container(
-        height: pageSize.height,
-        child: new PageableList<int>(
-          padding: new EdgeDims.symmetric(horizontal: 10.0),
-          items: pages,
-          itemBuilder: buildPage,
-          scrollDirection: ScrollDirection.horizontal,
-          itemExtent: pageSize.width,
-          pageChanged: (int page) {
-            currentPage = page;
-          }
-        )
-      );
-    }
-
-    tester.pumpFrame(builder);
+  test('Scroll left page 0 to page 1, itemsWrap: true', () {
+    WidgetTester tester = new WidgetTester();
+    currentPage = null;
+    tester.pumpFrame(() { return buildFrame(itemsWrap: true); });
+    expect(currentPage, isNull);
+    pageLeft(tester);
+    expect(currentPage, equals(1));
+  });
 
+  test('Scroll right from page 0 to page 5, itemsWrap: true', () {
+    WidgetTester tester = new WidgetTester();
+    currentPage = null;
+    tester.pumpFrame(() { return buildFrame(itemsWrap: true); });
     expect(currentPage, isNull);
-    new FakeAsync().run((async) {
-      tester.scroll(tester.findText('1'), new Offset(-300.0, 0.0));
-      // One frame to start the animation, a second to complete it.
-      tester.pumpFrame(builder);
-      tester.pumpFrame(builder, 5000.0);
-      async.elapse(new Duration(seconds: 5));
-      expect(currentPage, equals(2));
-    });
+    pageRight(tester);
+    expect(currentPage, equals(5));
   });
 }

packages/unit/test/widget/widget_tester.dart

@@ -117,8 +117,10 @@ class WidgetTester {
   void scroll(Widget widget, Offset offset, { int pointer: 1 }) {
     Point startLocation = getCenter(widget);
     Point endLocation = startLocation + offset;
-    HitTestResult result = _hitTest(startLocation);
     TestPointer p = new TestPointer(pointer);
+    // Events for the entire press-drag-release gesture are dispatched
+    // to the widgets "hit" by the pointer down event.
+    HitTestResult result = _hitTest(startLocation);
     _dispatchEvent(p.down(startLocation), result);
     _dispatchEvent(p.move(endLocation), result);
     _dispatchEvent(p.up(), result);