Handle changes to scrollDirection in ScrollableList et al

Changed the pageable_list.dart example: tapping on the toolbar changes
the scroll direction. This exposed some problems:

- Scrollable.syncFields() didn't update scrollDirection
- Viewport updated its RenderObject fields in the wrong order
- FixedHeightScrollable scrollDirection changes didn't update the scrollBehavior

There may be similar problems with VariableHeightList and ScrollableViewport.
I will fix those in a separate CL.

examples/widgets/page_scrollable.dart → examples/widgets/pageable_list.dart

@@ -24,13 +24,14 @@ class CardModel {
   Key get key => new Key.fromObjectIdentity(this);
 }
 
-class TestApp extends App {
+class PageableListApp extends App {
 
   static const TextStyle cardLabelStyle =
     const TextStyle(color: white, fontSize: 18.0, fontWeight: bold);
 
   List<CardModel> cardModels;
   Size pageSize = new Size(200.0, 200.0);
+  ScrollDirection scrollDirection = ScrollDirection.horizontal;
 
   void initState() {
     List<Size> cardSizes = [
@@ -55,6 +56,15 @@ class TestApp extends App {
     });
   }
 
+  EventDisposition handleToolbarTap(_) {
+    setState(() {
+      scrollDirection = (scrollDirection == ScrollDirection.vertical)
+        ? ScrollDirection.horizontal
+        : ScrollDirection.vertical;
+    });
+    return EventDisposition.processed;
+  }
+
   Widget buildCard(CardModel cardModel) {
     Widget card = new Card(
       color: cardModel.color,
@@ -65,9 +75,14 @@ class TestApp extends App {
         child: new Center(child: new Text(cardModel.label, style: cardLabelStyle))
       )
     );
+
+    BoxConstraints constraints = (scrollDirection == ScrollDirection.vertical)
+      ? new BoxConstraints.tightFor(height: pageSize.height)
+      : new BoxConstraints.tightFor(width: pageSize.width);
+
     return new Container(
       key: cardModel.key,
-      width: pageSize.width,
+      constraints: constraints,
       child: new Center(child: card)
     );
   }
@@ -76,8 +91,10 @@ class TestApp extends App {
     Widget list = new PageableList<CardModel>(
       items: cardModels,
       itemBuilder: buildCard,
-      scrollDirection: ScrollDirection.horizontal,
-      itemExtent: pageSize.width
+      scrollDirection: scrollDirection,
+      itemExtent: (scrollDirection == ScrollDirection.vertical)
+          ? pageSize.height
+          : pageSize.width
     );
 
     return new IconTheme(
@@ -91,7 +108,10 @@ class TestApp extends App {
         child: new TaskDescription(
           label: 'PageableList',
           child: new Scaffold(
-            toolbar: new ToolBar(center: new Text('PageableList Demo')),
+            toolbar: new Listener(
+              onGestureTap: handleToolbarTap,
+              child: new ToolBar(center: new Text('PageableList: ${scrollDirection}'))
+            ),
             body: new SizeObserver(
               callback: updatePageSize,
               child: new Container(
@@ -107,5 +127,5 @@ class TestApp extends App {
 }
 
 void main() {
-  runApp(new TestApp());
+  runApp(new PageableListApp());
 }

packages/flutter/lib/widgets/basic.dart

@@ -285,21 +285,22 @@ class Baseline extends OneChildRenderObjectWrapper {
 class Viewport extends OneChildRenderObjectWrapper {
   Viewport({
     Key key,
-    this.scrollOffset: Offset.zero,
     this.scrollDirection: ScrollDirection.vertical,
+    this.scrollOffset: Offset.zero,
     Widget child
   }) : super(key: key, child: child);
 
-  final Offset scrollOffset;
   final ScrollDirection scrollDirection;
+  final Offset scrollOffset;
 
-  RenderViewport createNode() => new RenderViewport(scrollOffset: scrollOffset, scrollDirection: scrollDirection);
+  RenderViewport createNode() => new RenderViewport(scrollDirection: scrollDirection, scrollOffset: scrollOffset);
   RenderViewport get renderObject => super.renderObject;
 
   void syncRenderObject(Viewport old) {
     super.syncRenderObject(old);
-    renderObject.scrollOffset = scrollOffset;
+    // Order dependency: RenderViewport validates scrollOffset based on scrollDirection.
     renderObject.scrollDirection = scrollDirection;
+    renderObject.scrollOffset = scrollOffset;
   }
 }
 

packages/flutter/lib/widgets/scrollable.dart

@@ -57,7 +57,7 @@ abstract class Scrollable extends StatefulComponent {
   }
 
   void syncFields(Scrollable source) {
-    scrollDirection == source.scrollDirection;
+    scrollDirection = source.scrollDirection;
   }
 
   double _scrollOffset = 0.0;
@@ -358,6 +358,7 @@ abstract class FixedHeightScrollable extends Scrollable {
 
   EdgeDims padding;
   double itemExtent;
+  Size containerSize = Size.zero;
 
   /// Subclasses must implement `get itemCount` to tell FixedHeightScrollable
   /// how many items there are in the list.
@@ -365,22 +366,32 @@ abstract class FixedHeightScrollable extends Scrollable {
   int _previousItemCount;
 
   void syncFields(FixedHeightScrollable source) {
-    if (padding != source.padding || itemExtent != source.itemExtent) {
-      padding = source.padding;
-      itemExtent = source.itemExtent;
-      _updateContentsExtent();
-    }
-    super.syncFields(source);
+    bool scrollBehaviorUpdateNeeded =
+      padding != source.padding ||
+      itemExtent != source.itemExtent ||
+      scrollDirection != source.scrollDirection;
+
+    padding = source.padding;
+    itemExtent = source.itemExtent;
+    super.syncFields(source); // update scrollDirection
+
+    if (scrollBehaviorUpdateNeeded)
+      _updateScrollBehavior();
   }
 
   ScrollBehavior createScrollBehavior() => new OverscrollBehavior();
   OverscrollBehavior get scrollBehavior => super.scrollBehavior;
 
-  double _containerExtent;
+  double get _containerExtent {
+    return scrollDirection == ScrollDirection.vertical
+      ? containerSize.height
+      : containerSize.width;
+  }
+
   void _handleSizeChanged(Size newSize) {
     setState(() {
-      _containerExtent = scrollDirection == ScrollDirection.vertical ? newSize.height : newSize.width;
-      scrollBehavior.containerSize = _containerExtent;
+      containerSize = newSize;
+      _updateScrollBehavior();
     });
   }
 
@@ -404,7 +415,9 @@ abstract class FixedHeightScrollable extends Scrollable {
     return new EdgeDims.only(top: padding.top, bottom: padding.bottom);
   }
 
-  void _updateContentsExtent() {
+  void _updateScrollBehavior() {
+    scrollBehavior.containerSize = _containerExtent;
+
     double contentsExtent = itemExtent * itemCount;
     if (padding != null)
       contentsExtent += _leadingPadding + _trailingPadding;
@@ -425,7 +438,7 @@ abstract class FixedHeightScrollable extends Scrollable {
   Widget buildContent() {
     if (itemCount != _previousItemCount) {
       _previousItemCount = itemCount;
-      _updateContentsExtent();
+      _updateScrollBehavior();
       _updateScrollOffset();
     }