Merge pull request #924 from HansMuller/custom_layout

Custom layouts use shouldRelayout() delegate methods instead of tokens

CustomMultiChildLayout and CustomOneChildLayout now use their delegate's shouldRelayout() method instead of a "token" to decide if layout is needed.

MultiChildLayoutDelegate and OnChildLayoutDelegate are now expected to be stateless, i.e. they'll typically be built each time their custom layout widget is built. If the identical layout delegate is provided to a new custom layout, layout will not happen.

Revised the bottom sheet implementation per the new custom layout classes. Removed a SizeObserver.

Fixes #899

packages/flutter/lib/src/material/bottom_sheet.dart

@@ -18,12 +18,11 @@ const double _kCloseProgressThreshold = 0.5;
 const Color _kTransparent = const Color(0x00000000);
 const Color _kBarrierColor = Colors.black54;
 
-class BottomSheet extends StatelessComponent {
+class BottomSheet extends StatefulComponent {
   BottomSheet({
     Key key,
     this.performance,
     this.onClosing,
-    this.childHeight,
     this.builder
   }) : super(key: key) {
     assert(onClosing != null);
@@ -34,37 +33,48 @@ class BottomSheet extends StatelessComponent {
   /// passive observer.
   final Performance performance;
   final VoidCallback onClosing;
-  final double childHeight;
   final WidgetBuilder builder;
 
+  _BottomSheetState createState() => new _BottomSheetState();
+
   static Performance createPerformanceController() {
     return new Performance(
       duration: _kBottomSheetDuration,
       debugLabel: 'BottomSheet'
     );
   }
+}
+
+class _BottomSheetState extends State<BottomSheet> {
+
+  final _childKey = new GlobalKey(debugLabel: 'BottomSheet child');
+
+  double get _childHeight {
+    final RenderBox renderBox = _childKey.currentContext.findRenderObject();
+    return renderBox.size.height;
+  }
 
-  bool get _dismissUnderway => performance.direction == AnimationDirection.reverse;
+  bool get _dismissUnderway => config.performance.direction == AnimationDirection.reverse;
 
   void _handleDragUpdate(double delta) {
     if (_dismissUnderway)
       return;
-    performance.progress -= delta / (childHeight ?? delta);
+    config.performance.progress -= delta / (_childHeight ?? delta);
   }
 
   void _handleDragEnd(Offset velocity) {
     if (_dismissUnderway)
       return;
     if (velocity.dy > _kMinFlingVelocity) {
-      double flingVelocity = -velocity.dy / childHeight;
-      performance.fling(velocity: flingVelocity);
+      double flingVelocity = -velocity.dy / _childHeight;
+      config.performance.fling(velocity: flingVelocity);
       if (flingVelocity < 0.0)
-        onClosing();
-    } else if (performance.progress < _kCloseProgressThreshold) {
-      performance.fling(velocity: -1.0);
-      onClosing();
+        config.onClosing();
+    } else if (config.performance.progress < _kCloseProgressThreshold) {
+      config.performance.fling(velocity: -1.0);
+      config.onClosing();
     } else {
-      performance.forward();
+      config.performance.forward();
     }
   }
 
@@ -73,7 +83,8 @@ class BottomSheet extends StatelessComponent {
       onVerticalDragUpdate: _handleDragUpdate,
       onVerticalDragEnd: _handleDragEnd,
       child: new Material(
-        child: builder(context)
+          key: _childKey,
+        child: config.builder(context)
       )
     );
   }
@@ -87,8 +98,9 @@ class BottomSheet extends StatelessComponent {
 // MODAL BOTTOM SHEETS
 
 class _ModalBottomSheetLayout extends OneChildLayoutDelegate {
-  // The distance from the bottom of the parent to the top of the BottomSheet child.
-  AnimatedValue<double> childTop = new AnimatedValue<double>(0.0);
+  _ModalBottomSheetLayout(this.progress);
+
+  final double progress;
 
   BoxConstraints getConstraintsForChild(BoxConstraints constraints) {
     return new BoxConstraints(
@@ -100,8 +112,11 @@ class _ModalBottomSheetLayout extends OneChildLayoutDelegate {
   }
 
   Point getPositionForChild(Size size, Size childSize) {
-    childTop.end = childSize.height;
-    return new Point(0.0, size.height - childTop.value);
+    return new Point(0.0, size.height - childSize.height * progress);
+  }
+
+  bool shouldRelayout(_ModalBottomSheetLayout oldDelegate) {
+    return progress != oldDelegate.progress;
   }
 }
 
@@ -114,24 +129,18 @@ class _ModalBottomSheet extends StatefulComponent {
 }
 
 class _ModalBottomSheetState extends State<_ModalBottomSheet> {
-
-  final _ModalBottomSheetLayout _layout = new _ModalBottomSheetLayout();
-
   Widget build(BuildContext context) {
     return new GestureDetector(
       onTap: () => Navigator.pop(context),
       child: new BuilderTransition(
         performance: config.route.performance,
-        variables: <AnimatedValue<double>>[_layout.childTop],
         builder: (BuildContext context) {
           return new ClipRect(
             child: new CustomOneChildLayout(
-              delegate: _layout,
-              token: _layout.childTop.value,
+              delegate: new _ModalBottomSheetLayout(config.route.performance.progress),
               child: new BottomSheet(
                 performance: config.route.performance,
                 onClosing: () => Navigator.pop(context),
-                childHeight: _layout.childTop.end,
                 builder: config.route.builder
               )
             )

packages/flutter/lib/src/material/scaffold.dart

@@ -95,8 +95,6 @@ class _ScaffoldLayout extends MultiChildLayoutDelegate {
   }
 }
 
-final _ScaffoldLayout _scaffoldLayout = new _ScaffoldLayout();
-
 class Scaffold extends StatefulComponent {
   Scaffold({
     Key key,
@@ -336,9 +334,8 @@ class ScaffoldState extends State<Scaffold> {
       ));
     }
 
-    return new CustomMultiChildLayout(children, delegate: _scaffoldLayout);
+    return new CustomMultiChildLayout(children, delegate: new _ScaffoldLayout());
   }
-
 }
 
 class ScaffoldFeatureController<T extends Widget> {
@@ -397,13 +394,6 @@ class _PersistentBottomSheetState extends State<_PersistentBottomSheet> {
       config.onDismissed();
   }
 
-  double _childHeight;
-  void _updateChildHeight(Size newSize) {
-    setState(() {
-      _childHeight = newSize.height;
-    });
-  }
-
   Widget build(BuildContext context) {
     return new AlignTransition(
       performance: config.performance,
@@ -412,8 +402,7 @@ class _PersistentBottomSheetState extends State<_PersistentBottomSheet> {
       child: new BottomSheet(
         performance: config.performance,
         onClosing: config.onClosing,
-        childHeight: _childHeight,
-        builder: (BuildContext context) => new SizeObserver(child: config.builder(context), onSizeChanged: _updateChildHeight)
+        builder: config.builder
       )
     );
   }

packages/flutter/lib/src/rendering/custom_layout.dart

@@ -91,6 +91,9 @@ abstract class MultiChildLayoutDelegate {
     }
   }
 
+  /// Override this method to return true when the children need to be laid out.
+  bool shouldRelayout(MultiChildLayoutDelegate oldDelegate) => true;
+
   /// Layout and position all children given this widget's size and the specified
   /// constraints. This method must apply layoutChild() to each child. It should
   /// specify the final position of each child with positionChild().
@@ -126,8 +129,9 @@ class RenderCustomMultiChildLayoutBox extends RenderBox
     assert(newDelegate != null);
     if (_delegate == newDelegate)
       return;
+    if (newDelegate.runtimeType != _delegate.runtimeType || newDelegate.shouldRelayout(_delegate))
+      markNeedsLayout();
     _delegate = newDelegate;
-    markNeedsLayout();
   }
 
   Size _getSize(BoxConstraints constraints) {

packages/flutter/lib/src/rendering/shifted_box.dart

@@ -257,6 +257,9 @@ class OneChildLayoutDelegate {
 
   /// Returns the position where the child should be placed given the size of this object and the size of the child.
   Point getPositionForChild(Size size, Size childSize) => Point.origin;
+
+  /// Override this method to return true when the child needs to be laid out.
+  bool shouldRelayout(OneChildLayoutDelegate oldDelegate) => true;
 }
 
 /// Defers the layout of its single child to a delegate.
@@ -280,8 +283,9 @@ class RenderCustomOneChildLayoutBox extends RenderShiftedBox {
     assert(newDelegate != null);
     if (_delegate == newDelegate)
       return;
+    if (newDelegate.runtimeType != _delegate.runtimeType || newDelegate.shouldRelayout(_delegate))
+      markNeedsLayout();
     _delegate = newDelegate;
-    markNeedsLayout();
   }
 
   Size _getSize(BoxConstraints constraints) {

packages/flutter/lib/src/widgets/basic.dart

@@ -386,27 +386,16 @@ class CustomOneChildLayout extends OneChildRenderObjectWidget {
   CustomOneChildLayout({
     Key key,
     this.delegate,
-    this.token,
     Widget child
   }) : super(key: key, child: child) {
     assert(delegate != null);
   }
 
-  /// A long-lived delegate that controls the layout of this widget.
-  ///
-  /// Whenever the delegate changes, we need to recompute the layout of this
-  /// widget, which means you might not want to create a new delegate instance
-  /// every time you build this widget. Instead, consider using a long-lived
-  /// deletate (perhaps held in a component's state) that you re-use every time
-  /// you build this widget.
   final OneChildLayoutDelegate delegate;
-  final Object token;
 
   RenderCustomOneChildLayoutBox createRenderObject() => new RenderCustomOneChildLayoutBox(delegate: delegate);
 
   void updateRenderObject(RenderCustomOneChildLayoutBox renderObject, CustomOneChildLayout oldWidget) {
-    if (oldWidget.token != token)
-      renderObject.markNeedsLayout();
     renderObject.delegate = delegate;
   }
 }
@@ -458,23 +447,19 @@ class LayoutId extends ParentDataWidget {
 class CustomMultiChildLayout extends MultiChildRenderObjectWidget {
   CustomMultiChildLayout(List<Widget> children, {
     Key key,
-    this.delegate,
-    this.token
+    this.delegate
   }) : super(key: key, children: children) {
     assert(delegate != null);
   }
 
   /// The delegate that controls the layout of the children.
   final MultiChildLayoutDelegate delegate;
-  final Object token;
 
   RenderCustomMultiChildLayoutBox createRenderObject() {
     return new RenderCustomMultiChildLayoutBox(delegate: delegate);
   }
 
   void updateRenderObject(RenderCustomMultiChildLayoutBox renderObject, CustomMultiChildLayout oldWidget) {
-    if (oldWidget.token != token)
-      renderObject.markNeedsLayout();
     renderObject.delegate = delegate;
   }
 }

packages/unit/test/widget/custom_multi_child_layout_test.dart

@@ -30,20 +30,32 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate {
       performLayoutIsChild = isChild('fred');
     }, returnsNormally);
   }
+
+  bool shouldRelayoutCalled = false;
+  bool shouldRelayoutValue = false;
+  bool shouldRelayout(_) {
+    shouldRelayoutCalled = true;
+    return shouldRelayoutValue;
+  }
 }
 
+Widget buildFrame(MultiChildLayoutDelegate delegate) {
+  return new Center(
+    child: new CustomMultiChildLayout([
+        new LayoutId(id: 0, child: new Container(width: 150.0, height: 100.0)),
+        new LayoutId(id: 1, child: new Container(width: 100.0, height: 200.0)),
+      ],
+      delegate: delegate
+    )
+  );
+}
+
+
 void main() {
   test('Control test for CustomMultiChildLayout', () {
     testWidgets((WidgetTester tester) {
       TestMultiChildLayoutDelegate delegate = new TestMultiChildLayoutDelegate();
-      tester.pumpWidget(new Center(
-        child: new CustomMultiChildLayout([
-          new LayoutId(id: 0, child: new Container(width: 150.0, height: 100.0)),
-          new LayoutId(id: 1, child: new Container(width: 100.0, height: 200.0)),
-        ],
-          delegate: delegate
-        )
-      ));
+      tester.pumpWidget(buildFrame(delegate));
 
       expect(delegate.getSizeConstraints.minWidth, 0.0);
       expect(delegate.getSizeConstraints.maxWidth, 800.0);
@@ -64,6 +76,31 @@ void main() {
     });
   });
 
+  test('Test MultiChildDelegate shouldRelayout method', () {
+    testWidgets((WidgetTester tester) {
+      TestMultiChildLayoutDelegate delegate = new TestMultiChildLayoutDelegate();
+      tester.pumpWidget(buildFrame(delegate));
+
+      // Layout happened because the delegate was set.
+      expect(delegate.performLayoutSize, isNotNull); // i.e. layout happened
+      expect(delegate.shouldRelayoutCalled, isFalse);
+
+      // Layout did not happen because shouldRelayout() returned false.
+      delegate = new TestMultiChildLayoutDelegate();
+      delegate.shouldRelayoutValue = false;
+      tester.pumpWidget(buildFrame(delegate));
+      expect(delegate.shouldRelayoutCalled, isTrue);
+      expect(delegate.performLayoutSize, isNull);
+
+      // Layout happened because shouldRelayout() returned true.
+      delegate = new TestMultiChildLayoutDelegate();
+      delegate.shouldRelayoutValue = true;
+      tester.pumpWidget(buildFrame(delegate));
+      expect(delegate.shouldRelayoutCalled, isTrue);
+      expect(delegate.performLayoutSize, isNotNull);
+    });
+  });
+
   test('Nested CustomMultiChildLayouts', () {
     testWidgets((WidgetTester tester) {
       TestMultiChildLayoutDelegate delegate = new TestMultiChildLayoutDelegate();

packages/unit/test/widget/custom_one_child_layout_test.dart

@@ -32,15 +32,24 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate {
     childSizeFromGetPositionForChild = childSize;
     return Point.origin;
   }
+
+  bool shouldRelayoutCalled = false;
+  bool shouldRelayoutValue = false;
+  bool shouldRelayout(_) {
+    shouldRelayoutCalled = true;
+    return shouldRelayoutValue;
+  }
+}
+
+Widget buildFrame(delegate) {
+  return new Center(child: new CustomOneChildLayout(delegate: delegate, child: new Container()));
 }
 
 void main() {
   test('Control test for CustomOneChildLayout', () {
     testWidgets((WidgetTester tester) {
       TestOneChildLayoutDelegate delegate = new TestOneChildLayoutDelegate();
-      tester.pumpWidget(new Center(
-        child: new CustomOneChildLayout(delegate: delegate, child: new Container())
-      ));
+      tester.pumpWidget(buildFrame(delegate));
 
       expect(delegate.constraintsFromGetSize.minWidth, 0.0);
       expect(delegate.constraintsFromGetSize.maxWidth, 800.0);
@@ -59,4 +68,30 @@ void main() {
       expect(delegate.childSizeFromGetPositionForChild.height, 400.0);
     });
   });
+
+  test('Test OneChildDelegate shouldRelayout method', () {
+    testWidgets((WidgetTester tester) {
+      TestOneChildLayoutDelegate delegate = new TestOneChildLayoutDelegate();
+      tester.pumpWidget(buildFrame(delegate));
+
+      // Layout happened because the delegate was set.
+      expect(delegate.constraintsFromGetConstraintsForChild, isNotNull); // i.e. layout happened
+      expect(delegate.shouldRelayoutCalled, isFalse);
+
+      // Layout did not happen because shouldRelayout() returned false.
+      delegate = new TestOneChildLayoutDelegate();
+      delegate.shouldRelayoutValue = false;
+      tester.pumpWidget(buildFrame(delegate));
+      expect(delegate.shouldRelayoutCalled, isTrue);
+      expect(delegate.constraintsFromGetConstraintsForChild, isNull);
+
+      // Layout happened because shouldRelayout() returned true.
+      delegate = new TestOneChildLayoutDelegate();
+      delegate.shouldRelayoutValue = true;
+      tester.pumpWidget(buildFrame(delegate));
+      expect(delegate.shouldRelayoutCalled, isTrue);
+      expect(delegate.constraintsFromGetConstraintsForChild, isNotNull);
+    });
+  });
+
 }