ExpansionPanel animation fixes (#13032)

Previously, ExpansionPanel would do weird things if interacted with
when it was already animating. This is fixed and there's now a test.

Also:

 * Minor fixes to make the gallery work in RTL, not that there's
   any way to see that without hard-coding the framework to RTL.
   But at least I'll be less annoyed when doing that.

 * Some trivial code and documentation cleanup.

examples/flutter_gallery/lib/gallery/home.dart

@@ -142,8 +142,8 @@ class GalleryHomeState extends State<GalleryHome> with SingleTickerProviderState
           new MergeSemantics(
             child: new Container(
               height: 48.0,
-              padding: const EdgeInsets.only(left: 16.0),
-              alignment: Alignment.centerLeft,
+              padding: const EdgeInsetsDirectional.only(start: 16.0),
+              alignment: AlignmentDirectional.centerStart,
               child: new Text(galleryItem.category, style: headerStyle)
             ),
           )

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

@@ -64,7 +64,7 @@ typedef Widget ExpansionPanelHeaderBuilder(BuildContext context, bool isExpanded
 class ExpansionPanel {
   /// Creates an expansion panel to be used as a child for [ExpansionPanelList].
   ///
-  /// None of the arguments can be null.
+  /// The [headerBuilder], [body], and [isExpanded] arguments must not be null.
   ExpansionPanel({
     @required this.headerBuilder,
     @required this.body,
@@ -133,9 +133,9 @@ class ExpansionPanelList extends StatelessWidget {
       vertical: _kPanelHeaderExpandedHeight - _kPanelHeaderCollapsedHeight
     );
 
-    for (int i = 0; i < children.length; i += 1) {
-      if (_isChildExpanded(i) && i != 0 && !_isChildExpanded(i - 1))
-        items.add(new MaterialGap(key: new _SaltedKey<BuildContext, int>(context, i * 2 - 1)));
+    for (int index = 0; index < children.length; index += 1) {
+      if (_isChildExpanded(index) && index != 0 && !_isChildExpanded(index - 1))
+        items.add(new MaterialGap(key: new _SaltedKey<BuildContext, int>(context, index * 2 - 1)));
 
       final Row header = new Row(
         children: <Widget>[
@@ -143,58 +143,57 @@ class ExpansionPanelList extends StatelessWidget {
             child: new AnimatedContainer(
               duration: animationDuration,
               curve: Curves.fastOutSlowIn,
-              margin: _isChildExpanded(i) ? kExpandedEdgeInsets : EdgeInsets.zero,
+              margin: _isChildExpanded(index) ? kExpandedEdgeInsets : EdgeInsets.zero,
               child: new SizedBox(
                 height: _kPanelHeaderCollapsedHeight,
-                child: children[i].headerBuilder(
+                child: children[index].headerBuilder(
                   context,
-                  children[i].isExpanded
-                )
-              )
-            )
+                  children[index].isExpanded,
+                ),
+              ),
+            ),
           ),
           new Container(
             margin: const EdgeInsetsDirectional.only(end: 8.0),
             child: new ExpandIcon(
-              isExpanded: _isChildExpanded(i),
+              isExpanded: _isChildExpanded(index),
               padding: const EdgeInsets.all(16.0),
               onPressed: (bool isExpanded) {
-                if (expansionCallback != null) {
-                  expansionCallback(i, isExpanded);
-                }
-              }
-            )
-          )
-        ]
+                if (expansionCallback != null)
+                  expansionCallback(index, isExpanded);
+              },
+            ),
+          ),
+        ],
       );
 
       items.add(
         new MaterialSlice(
-          key: new _SaltedKey<BuildContext, int>(context, i * 2),
+          key: new _SaltedKey<BuildContext, int>(context, index * 2),
           child: new Column(
             children: <Widget>[
               header,
               new AnimatedCrossFade(
                 firstChild: new Container(height: 0.0),
-                secondChild: children[i].body,
+                secondChild: children[index].body,
                 firstCurve: const Interval(0.0, 0.6, curve: Curves.fastOutSlowIn),
                 secondCurve: const Interval(0.4, 1.0, curve: Curves.fastOutSlowIn),
                 sizeCurve: Curves.fastOutSlowIn,
-                crossFadeState: _isChildExpanded(i) ? CrossFadeState.showSecond : CrossFadeState.showFirst,
+                crossFadeState: _isChildExpanded(index) ? CrossFadeState.showSecond : CrossFadeState.showFirst,
                 duration: animationDuration,
-              )
-            ]
-          )
-        )
+              ),
+            ],
+          ),
+        ),
       );
 
-      if (_isChildExpanded(i) && i != children.length - 1)
-        items.add(new MaterialGap(key: new _SaltedKey<BuildContext, int>(context, i * 2 + 1)));
+      if (_isChildExpanded(index) && index != children.length - 1)
+        items.add(new MaterialGap(key: new _SaltedKey<BuildContext, int>(context, index * 2 + 1)));
     }
 
     return new MergeableMaterial(
       hasDividers: true,
-      children: items
+      children: items,
     );
   }
 }

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

@@ -6,7 +6,7 @@ import 'package:flutter/widgets.dart';
 
 /// Identifiers for the supported material design icons.
 ///
-/// Use with with the [Icon] class to show specific icons.
+/// Use with the [Icon] class to show specific icons.
 ///
 /// Icons are identified by their name as listed below.
 ///

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

@@ -456,6 +456,9 @@ class _MergeableMaterialState extends State<MergeableMaterial> with TickerProvid
   }
 
   BorderRadius _borderRadius(int index, bool start, bool end) {
+    assert(kMaterialEdges[MaterialType.card].topLeft == kMaterialEdges[MaterialType.card].topRight);
+    assert(kMaterialEdges[MaterialType.card].topLeft == kMaterialEdges[MaterialType.card].bottomLeft);
+    assert(kMaterialEdges[MaterialType.card].topLeft == kMaterialEdges[MaterialType.card].bottomRight);
     final Radius cardRadius = kMaterialEdges[MaterialType.card].topLeft;
 
     Radius startRadius = Radius.zero;

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

@@ -16,32 +16,37 @@ enum RenderAnimatedSizeState {
   /// The initial state, when we do not yet know what the starting and target
   /// sizes are to animate.
   ///
-  /// Next possible state is [stable].
+  /// The next state is [stable].
   start,
 
   /// At this state the child's size is assumed to be stable and we are either
   /// animating, or waiting for the child's size to change.
   ///
-  /// Next possible state is [changed].
+  /// If the child's size changes, the state will become [changed]. Otherwise,
+  /// it remains [stable].
   stable,
 
   /// At this state we know that the child has changed once after being assumed
   /// [stable].
   ///
-  /// Next possible states are:
+  /// The next state will be one of:
   ///
-  /// - [stable] - if the child's size stabilized immediately, this is a signal
-  /// for us to begin animating the size towards the child's new size.
-  /// - [unstable] - if the child's size continues to change, we assume it is
-  /// not stable and enter the [unstable] state.
+  /// * [stable] if the child's size stabilized immediately. This is a signal
+  ///   for the render object to begin animating the size towards the child's new
+  ///   size.
+  ///
+  /// * [unstable] if the child's size continues to change.
   changed,
 
-  /// At this state the child's size is assumed to be unstable.
+  /// At this state the child's size is assumed to be unstable (changing each
+  /// frame).
   ///
-  /// Instead of chasing the child's size in this state we tightly track the
-  /// child's size until it stabilizes.
+  /// Instead of chasing the child's size in this state, the render object
+  /// tightly tracks the child's size until it stabilizes.
   ///
-  /// Next possible state is [stable].
+  /// The render object remains in this state until a frame where the child's
+  /// size remains the same as the previous frame. At that time, the next state
+  /// is [stable].
   unstable,
 }
 
@@ -144,7 +149,6 @@ class RenderAnimatedSize extends RenderAligningShiftedBox {
   @override
   void detach() {
     _controller.stop();
-    _state = RenderAnimatedSizeState.start;
     super.detach();
   }
 
@@ -212,12 +216,15 @@ class RenderAnimatedSize extends RenderAligningShiftedBox {
   /// animation.
   void _layoutStable() {
     if (_sizeTween.end != child.size) {
+      _sizeTween.begin = size;
       _sizeTween.end = debugAdoptSize(child.size);
       _restartAnimation();
       _state = RenderAnimatedSizeState.changed;
     } else if (_controller.value == _controller.upperBound) {
       // Animation finished. Reset target sizes.
       _sizeTween.begin = _sizeTween.end = debugAdoptSize(child.size);
+    } else if (!_controller.isAnimating) {
+      _controller.forward(); // resume the animation after being detached
     }
   }
 
@@ -236,6 +243,8 @@ class RenderAnimatedSize extends RenderAligningShiftedBox {
     } else {
       // Child size stabilized.
       _state = RenderAnimatedSizeState.stable;
+      if (!_controller.isAnimating)
+        _controller.forward(); // resume the animation after being detached
     }
   }
 

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

@@ -604,6 +604,10 @@ class RenderConstrainedOverflowBox extends RenderAligningShiftedBox {
 ///    passes its original constraints through to its child, which it allows to
 ///    overflow.
 class RenderUnconstrainedBox extends RenderAligningShiftedBox with DebugOverflowIndicatorMixin {
+  /// Create a render object that sizes itself to the child but does not
+  /// pass the [constraints] down to that child.
+  ///
+  /// The [alignment] must not be null.
   RenderUnconstrainedBox({
     @required AlignmentGeometry alignment,
     @required TextDirection textDirection,

packages/flutter/lib/src/services/image_stream.dart

@@ -363,7 +363,7 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter {
   int _framesEmitted;
   Timer _timer;
 
-  void _handleCodecReady(ui.Codec codec){
+  void _handleCodecReady(ui.Codec codec) {
     _codec = codec;
     _decodeNextFrameAndSchedule();
   }

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

@@ -67,8 +67,8 @@ typedef Widget AnimatedCrossFadeBuilder(Widget topChild, Key topChildKey, Widget
 /// [firstCurve] and [secondCurve] represent the opacity curves of the two
 /// children. The [firstCurve] is inverted, i.e. it fades out when providing a
 /// growing curve like [Curves.linear]. The [sizeCurve] is the curve used to
-/// animated between the size of the fading out child and the size of the fading
-/// in child.
+/// animate between the size of the fading-out child and the size of the
+/// fading-in child.
 ///
 /// This widget is intended to be used to fade a pair of widgets with the same
 /// width. In the case where the two children have different heights, the

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

@@ -88,7 +88,7 @@ abstract class ScrollNotification extends LayoutChangedNotification with Viewpor
     @required this.context,
   });
 
-  // A description of a [Scrollable]'s contents, useful for modeling the state
+  /// A description of a [Scrollable]'s contents, useful for modeling the state
   /// of its viewport.
   final ScrollMetrics metrics;
 

packages/flutter/test/animation/animation_controller_test.dart

@@ -289,8 +289,8 @@ void main() {
     final AnimationController controller = new AnimationController(
       vsync: const TestVSync(),
     );
-    expect((){ controller.repeat(); }, throwsFlutterError);
-    expect((){ controller.repeat(period: null); }, throwsFlutterError);
+    expect(() { controller.repeat(); }, throwsFlutterError);
+    expect(() { controller.repeat(period: null); }, throwsFlutterError);
   });
 
   test('Do not animate if already at target', () {

packages/flutter/test/cupertino/scaffold_test.dart

@@ -124,7 +124,7 @@ void main() {
                                       child: new Center(
                                         child: new CupertinoButton(
                                           child: const Text('Back'),
-                                          onPressed: (){
+                                          onPressed: () {
                                             Navigator.of(context).pop();
                                           },
                                         ),

packages/flutter/test/material/dropdown_test.dart

@@ -13,7 +13,7 @@ import '../widgets/semantics_tester.dart';
 const List<String> menuItems = const <String>['one', 'two', 'three', 'four'];
 
 final Type dropdownButtonType = new DropdownButton<String>(
-  onChanged: (_){ },
+  onChanged: (_) { },
   items: const <DropdownMenuItem<String>>[]
 ).runtimeType;
 

packages/flutter/test/material/expansion_panel_test.dart

@@ -71,6 +71,7 @@ void main() {
     box = tester.renderObject(find.byType(ExpansionPanelList));
     expect(box.size.height - oldHeight, greaterThanOrEqualTo(100.0)); // 100 + some margin
   });
+
   testWidgets('Multiple Panel List test', (WidgetTester tester) async {
     await tester.pumpWidget(
       new MaterialApp(
@@ -90,7 +91,7 @@ void main() {
             new ExpansionPanelList(
               children: <ExpansionPanel>[
                 new ExpansionPanel(
-                  headerBuilder: (BuildContext context, bool isExpanded){
+                  headerBuilder: (BuildContext context, bool isExpanded) {
                     return new Text(isExpanded ? 'D' : 'C');
                   },
                   body: const SizedBox(height: 100.0),
@@ -109,4 +110,90 @@ void main() {
     expect(find.text('C'), findsNothing);
     expect(find.text('D'), findsOneWidget);
   });
+
+  testWidgets('Open/close animations', (WidgetTester tester) async {
+    const Duration kSizeAnimationDuration = const Duration(milliseconds: 1000);
+    // The MaterialGaps animate in using kThemeAnimationDuration (hardcoded),
+    // which should be less than our test size animation length. So we can assume that they
+    // appear immediately. Here we just verify that our assumption is true.
+    expect(kThemeAnimationDuration, lessThan(kSizeAnimationDuration ~/ 2));
+
+    Widget build(bool a, bool b, bool c) {
+      return new MaterialApp(
+        home: new Column(
+          children: <Widget>[
+            new ExpansionPanelList(
+              animationDuration: kSizeAnimationDuration,
+              children: <ExpansionPanel>[
+                new ExpansionPanel(
+                  headerBuilder: (BuildContext context, bool isExpanded) => const Placeholder(),
+                  body: const SizedBox(height: 100.0, child: const Placeholder()),
+                  isExpanded: a,
+                ),
+                new ExpansionPanel(
+                  headerBuilder: (BuildContext context, bool isExpanded) => const Placeholder(),
+                  body: const SizedBox(height: 100.0, child: const Placeholder()),
+                  isExpanded: b,
+                ),
+                new ExpansionPanel(
+                  headerBuilder: (BuildContext context, bool isExpanded) => const Placeholder(),
+                  body: const SizedBox(height: 100.0, child: const Placeholder()),
+                  isExpanded: c,
+                ),
+              ],
+            ),
+          ],
+        ),
+      );
+    }
+
+    await tester.pumpWidget(build(false, false, false));
+    expect(tester.renderObjectList(find.byType(AnimatedSize)), hasLength(3));
+    expect(tester.getRect(find.byType(AnimatedSize).at(0)), new Rect.fromLTWH(0.0, 56.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(1)), new Rect.fromLTWH(0.0, 113.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(2)), new Rect.fromLTWH(0.0, 170.0, 800.0, 0.0));
+
+    await tester.pump(const Duration(milliseconds: 200));
+    expect(tester.getRect(find.byType(AnimatedSize).at(0)), new Rect.fromLTWH(0.0, 56.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(1)), new Rect.fromLTWH(0.0, 113.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(2)), new Rect.fromLTWH(0.0, 170.0, 800.0, 0.0));
+
+    await tester.pumpWidget(build(false, true, false));
+    expect(tester.getRect(find.byType(AnimatedSize).at(0)), new Rect.fromLTWH(0.0, 56.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(1)), new Rect.fromLTWH(0.0, 113.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(2)), new Rect.fromLTWH(0.0, 170.0, 800.0, 0.0));
+
+    await tester.pump(kSizeAnimationDuration ~/ 2);
+    expect(tester.getRect(find.byType(AnimatedSize).at(0)), new Rect.fromLTWH(0.0, 56.0, 800.0, 0.0));
+    final Rect rect1 = tester.getRect(find.byType(AnimatedSize).at(1));
+    expect(rect1.left, 0.0);
+    expect(rect1.top, inExclusiveRange(113.0, 113.0 + 16.0 + 32.0)); // 16.0 material gap, plus 16.0 top and bottom margins added to the header
+    expect(rect1.width, 800.0);
+    expect(rect1.height, inExclusiveRange(0.0, 100.0));
+    final Rect rect2 = tester.getRect(find.byType(AnimatedSize).at(2));
+    expect(rect2, new Rect.fromLTWH(0.0, rect1.bottom + 16.0 + 56.0, 800.0, 0.0)); // the 16.0 comes from the MaterialGap being introduced, the 56.0 is the header height.
+
+    await tester.pumpWidget(build(false, false, false));
+    expect(tester.getRect(find.byType(AnimatedSize).at(0)), new Rect.fromLTWH(0.0, 56.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(1)), rect1);
+    expect(tester.getRect(find.byType(AnimatedSize).at(2)), rect2);
+
+    await tester.pumpWidget(build(false, false, true));
+    expect(tester.getRect(find.byType(AnimatedSize).at(0)), new Rect.fromLTWH(0.0, 56.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(1)), rect1);
+    expect(tester.getRect(find.byType(AnimatedSize).at(2)), rect2);
+
+    // a few no-op pumps to make sure there's nothing fishy going on
+    await tester.pump();
+    await tester.pump();
+    await tester.pump();
+    expect(tester.getRect(find.byType(AnimatedSize).at(0)), new Rect.fromLTWH(0.0, 56.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(1)), rect1);
+    expect(tester.getRect(find.byType(AnimatedSize).at(2)), rect2);
+
+    await tester.pumpAndSettle();
+    expect(tester.getRect(find.byType(AnimatedSize).at(0)), new Rect.fromLTWH(0.0, 56.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(1)), new Rect.fromLTWH(0.0, 56.0 + 1.0 + 56.0, 800.0, 0.0));
+    expect(tester.getRect(find.byType(AnimatedSize).at(2)), new Rect.fromLTWH(0.0, 56.0 + 1.0 + 56.0 + 16.0 + 16.0 + 48.0 + 16.0, 800.0, 100.0));
+  });
 }

packages/flutter/test/material/persistent_bottom_sheet_test.dart

@@ -30,7 +30,7 @@ void main() {
     await tester.pump();
     expect(buildCount, equals(1));
 
-    bottomSheet.setState((){ });
+    bottomSheet.setState(() { });
     await tester.pump();
     expect(buildCount, equals(2));
   });

packages/flutter/test/widgets/dispose_ancestor_lookup_test.dart

@@ -98,7 +98,7 @@ void main() {
     await tester.pumpWidget(
       new TestWidget((BuildContext context) {
         disposeCalled = true;
-        context.visitAncestorElements((Element element){ });
+        context.visitAncestorElements((Element element) { });
       }),
     );
     await tester.pumpWidget(new Container());