From c73b8a7cf63455189e9dc005010f2c9b34497420 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 12 Apr 2018 16:17:26 -0700 Subject: [PATCH] Fix a bug in the AnimatedChildSwitcher, add builders. (#16250) This fixes a rendering problem in the AnimatedChildSwitcher where it would add a new "previous" child each time it rebuilt, and if you did it fast enough, all of them would disappear from the page. It also expands the API for AnimatedChildSwitcher to allow you to specify your own transition and/or layout builder for the transition. Fixes #16226 --- packages/flutter/lib/src/rendering/stack.dart | 5 + .../src/widgets/animated_child_switcher.dart | 288 ++++++++++++++---- .../lib/src/widgets/animated_cross_fade.dart | 2 + .../flutter/test/rendering/stack_test.dart | 11 + .../widgets/animated_child_switcher_test.dart | 180 ++++++++++- 5 files changed, 428 insertions(+), 58 deletions(-) diff --git a/packages/flutter/lib/src/rendering/stack.dart b/packages/flutter/lib/src/rendering/stack.dart index 34121bf9d8..91b214f4a2 100644 --- a/packages/flutter/lib/src/rendering/stack.dart +++ b/packages/flutter/lib/src/rendering/stack.dart @@ -480,6 +480,11 @@ class RenderStack extends RenderBox assert(_resolvedAlignment != null); _hasVisualOverflow = false; bool hasNonPositionedChildren = false; + if (childCount == 0) { + size = constraints.biggest; + assert(size.isFinite); + return; + } double width = constraints.minWidth; double height = constraints.minHeight; diff --git a/packages/flutter/lib/src/widgets/animated_child_switcher.dart b/packages/flutter/lib/src/widgets/animated_child_switcher.dart index 8b42690472..bb3183521b 100644 --- a/packages/flutter/lib/src/widgets/animated_child_switcher.dart +++ b/packages/flutter/lib/src/widgets/animated_child_switcher.dart @@ -10,21 +10,110 @@ import 'framework.dart'; import 'ticker_provider.dart'; import 'transitions.dart'; +// Internal representation of a child that, now or in the past, was set on the +// AnimatedChildSwitcher.child field, but is now in the process of +// transitioning. The internal representation includes fields that we don't want +// to expose to the public API (like the controller). class _AnimatedChildSwitcherChildEntry { - _AnimatedChildSwitcherChildEntry(this.widget, this.controller, this.animation); + _AnimatedChildSwitcherChildEntry({ + @required this.animation, + @required this.transition, + @required this.controller, + @required this.widgetChild, + }) : assert(animation != null), + assert(transition != null), + assert(controller != null); - Widget widget; - - final AnimationController controller; final Animation animation; + + // The currently built transition for this child. + Widget transition; + + // The animation controller for the child's transition. + final AnimationController controller; + + // The widget's child at the time this entry was created or updated. + Widget widgetChild; } -/// A widget that automatically does a [FadeTransition] between a new widget and +/// Signature for builders used to generate custom transitions for +/// [AnimatedChildSwitcher]. +/// +/// The [child] should be transitioning in when the [animation] is running in +/// the forward direction. +/// +/// The function should return a widget which wraps the given [child]. It may +/// also use the [animation] to inform its transition. It must not return null. +typedef Widget AnimatedChildSwitcherTransitionBuilder(Widget child, Animation animation); + +/// Signature for builders used to generate custom layouts for +/// [AnimatedChildSwitcher]. +/// +/// The function should return a widget which contains the given children, laid +/// out as desired. It must not return null. +typedef Widget AnimatedChildSwitcherLayoutBuilder(List children); + +/// A widget that by default does a [FadeTransition] between a new widget and /// the widget previously set on the [AnimatedChildSwitcher] as a child. /// -/// More than one previous child can exist and be fading out while the newest -/// one is fading in if they are swapped fast enough (i.e. before [duration] -/// elapses). +/// If they are swapped fast enough (i.e. before [duration] elapses), more than +/// one previous child can exist and be transitioning out while the newest one +/// is transitioning in. +/// +/// If the "new" child is the same widget type as the "old" child, but with +/// different parameters, then [AnimatedChildSwitcher] will *not* do a +/// transition between them, since as far as the framework is concerned, they +/// are the same widget, and the existing widget can be updated with the new +/// parameters. If you wish to force the transition to occur, set a [Key] +/// (typically a [ValueKey] taking any widget data that would change the visual +/// appearance of the widget) on each child widget that you wish to be +/// considered unique. +/// +/// ## Sample code +/// +/// ```dart +/// class ClickCounter extends StatefulWidget { +/// const ClickCounter({Key key}) : super(key: key); +/// +/// @override +/// _ClickCounterState createState() => new _ClickCounterState(); +/// } +/// +/// class _ClickCounterState extends State { +/// int _count = 0; +/// +/// @override +/// Widget build(BuildContext context) { +/// return new Material( +/// child: Column( +/// mainAxisAlignment: MainAxisAlignment.center, +/// children: [ +/// new AnimatedChildSwitcher( +/// duration: const Duration(milliseconds: 200), +/// transitionBuilder: (Widget child, Animation animation) { +/// return new ScaleTransition(child: child, scale: animation); +/// }, +/// child: new Text( +/// '$_count', +/// // Must have this key to build a unique widget when _count changes. +/// key: new ValueKey(_count), +/// textScaleFactor: 3.0, +/// ), +/// ), +/// new RaisedButton( +/// child: new Text('Click!'), +/// onPressed: () { +/// setState(() { +/// _count += 1; +/// }); +/// }, +/// ), +/// ], +/// ), +/// ); +/// } +/// } +/// ``` /// /// See also: /// @@ -32,17 +121,23 @@ class _AnimatedChildSwitcherChildEntry { /// interpolates their sizes, and is reversible. /// * [FadeTransition] which [AnimatedChildSwitcher] uses to perform the transition. class AnimatedChildSwitcher extends StatefulWidget { - /// The [duration], [switchInCurve], and [switchOutCurve] parameters must not - /// be null. + /// Creates an [AnimatedChildSwitcher]. + /// + /// The [duration], [transitionBuilder], [layoutBuilder], [switchInCurve], and + /// [switchOutCurve] parameters must not be null. const AnimatedChildSwitcher({ Key key, this.child, + @required this.duration, this.switchInCurve: Curves.linear, this.switchOutCurve: Curves.linear, - @required this.duration, - }) : assert(switchInCurve != null), + this.transitionBuilder: AnimatedChildSwitcher.defaultTransitionBuilder, + this.layoutBuilder: AnimatedChildSwitcher.defaultLayoutBuilder, + }) : assert(duration != null), + assert(switchInCurve != null), assert(switchOutCurve != null), - assert(duration != null), + assert(transitionBuilder != null), + assert(layoutBuilder != null), super(key: key); /// The current child widget to display. If there was a previous child, @@ -53,17 +148,71 @@ class AnimatedChildSwitcher extends StatefulWidget { /// [duration]. final Widget child; - /// The animation curve to use when fading in the current widget. + /// The duration of the transition from the old [child] value to the new one. + final Duration duration; + + /// The animation curve to use when transitioning in [child]. final Curve switchInCurve; - /// The animation curve to use when fading out the previous widgets. + /// The animation curve to use when transitioning the previous [child] out. final Curve switchOutCurve; - /// The duration over which to perform the cross fade using [FadeTransition]. - final Duration duration; + /// A function that wraps the new [child] with an animation that transitions + /// the [child] in when the animation runs in the forward direction and out + /// when the animation runs in the reverse direction. + /// + /// The default is [AnimatedChildSwitcher.defaultTransitionBuilder]. + /// + /// See also: + /// + /// * [AnimatedChildSwitcherTransitionBuilder] for more information about + /// how a transition builder should function. + final AnimatedChildSwitcherTransitionBuilder transitionBuilder; + + /// A function that wraps all of the children that are transitioning out, and + /// the [child] that's transitioning in, with a widget that lays all of them + /// out. + /// + /// The default is [AnimatedChildSwitcher.defaultLayoutBuilder]. + /// + /// See also: + /// + /// * [AnimatedChildSwitcherLayoutBuilder] for more information about + /// how a layout builder should function. + final AnimatedChildSwitcherLayoutBuilder layoutBuilder; @override _AnimatedChildSwitcherState createState() => new _AnimatedChildSwitcherState(); + + /// The default transition algorithm used by [AnimatedChildSwitcher]. + /// + /// The new child is given a [FadeTransition] which increases opacity as + /// the animation goes from 0.0 to 1.0, and decreases when the animation is + /// reversed. + /// + /// The default value for the [transitionBuilder], an + /// [AnimatedChildSwitcherTransitionBuilder] function. + static Widget defaultTransitionBuilder(Widget child, Animation animation) { + return new FadeTransition( + opacity: animation, + child: child, + ); + } + + /// The default layout algorithm used by [AnimatedChildSwitcher]. + /// + /// The new child is placed in a [Stack] that sizes itself to match the + /// largest of the child or a previous child. The children are centered on + /// each other. + /// + /// This is the default value for [layoutBuilder]. It implements + /// [AnimatedChildSwitcherLayoutBuilder]. + static Widget defaultLayoutBuilder(List children) { + return new Stack( + children: children, + alignment: Alignment.center, + ); + } } class _AnimatedChildSwitcherState extends State with TickerProviderStateMixin { @@ -73,10 +222,48 @@ class _AnimatedChildSwitcherState extends State with Tick @override void initState() { super.initState(); - addEntry(false); + _addEntry(animate: false); } - void addEntry(bool animate) { + Widget _generateTransition(Animation animation) { + return new KeyedSubtree( + key: new UniqueKey(), + child: widget.transitionBuilder(widget.child, animation), + ); + } + + _AnimatedChildSwitcherChildEntry _newEntry({ + @required AnimationController controller, + @required Animation animation, + }) { + final _AnimatedChildSwitcherChildEntry entry = new _AnimatedChildSwitcherChildEntry( + widgetChild: widget.child, + transition: _generateTransition(animation), + animation: animation, + controller: controller, + ); + animation.addStatusListener((AnimationStatus status) { + if (status == AnimationStatus.dismissed) { + assert(_children.contains(entry)); + setState(() { + _children.remove(entry); + }); + controller.dispose(); + } + }); + return entry; + } + + void _addEntry({@required bool animate}) { + if (widget.child == null) { + if (animate && _currentChild != null) { + _currentChild.controller.reverse(); + assert(!_children.contains(_currentChild)); + _children.add(_currentChild); + } + _currentChild = null; + return; + } final AnimationController controller = new AnimationController( duration: widget.duration, vsync: this, @@ -84,6 +271,7 @@ class _AnimatedChildSwitcherState extends State with Tick if (animate) { if (_currentChild != null) { _currentChild.controller.reverse(); + assert(!_children.contains(_currentChild)); _children.add(_currentChild); } controller.forward(); @@ -97,21 +285,7 @@ class _AnimatedChildSwitcherState extends State with Tick curve: widget.switchInCurve, reverseCurve: widget.switchOutCurve, ); - final _AnimatedChildSwitcherChildEntry entry = new _AnimatedChildSwitcherChildEntry( - widget.child, - controller, - animation, - ); - animation.addStatusListener((AnimationStatus status) { - if (status == AnimationStatus.dismissed) { - assert(_children.contains(entry)); - setState(() { - _children.remove(entry); - }); - controller.dispose(); - } - }); - _currentChild = entry; + _currentChild = _newEntry(controller: controller, animation: animation); } @override @@ -125,31 +299,33 @@ class _AnimatedChildSwitcherState extends State with Tick super.dispose(); } + bool get hasNewChild => widget.child != null; + bool get hasOldChild => _currentChild != null; + + @override + void didUpdateWidget(AnimatedChildSwitcher oldWidget) { + super.didUpdateWidget(oldWidget); + if (hasNewChild != hasOldChild || hasNewChild && + !Widget.canUpdate(widget.child, _currentChild.widgetChild)) { + _addEntry(animate: true); + } else { + if (_currentChild != null) { + _currentChild.widgetChild = widget.child; + _currentChild.transition = _generateTransition(_currentChild.animation); + } + } + } + @override Widget build(BuildContext context) { - if (widget.child != _currentChild.widget) { - addEntry(true); - } - final List children = []; - for (_AnimatedChildSwitcherChildEntry child in _children) { - children.add( - new FadeTransition( - opacity: child.animation, - child: child.widget, - ), - ); - } + final List children = _children.map( + (_AnimatedChildSwitcherChildEntry entry) { + return entry.transition; + }, + ).toList(); if (_currentChild != null) { - children.add( - new FadeTransition( - opacity: _currentChild.animation, - child: _currentChild.widget, - ), - ); + children.add(_currentChild.transition); } - return new Stack( - children: children, - alignment: Alignment.center, - ); + return widget.layoutBuilder(children); } } diff --git a/packages/flutter/lib/src/widgets/animated_cross_fade.dart b/packages/flutter/lib/src/widgets/animated_cross_fade.dart index 2102d20d44..4781ee5386 100644 --- a/packages/flutter/lib/src/widgets/animated_cross_fade.dart +++ b/packages/flutter/lib/src/widgets/animated_cross_fade.dart @@ -100,6 +100,8 @@ typedef Widget AnimatedCrossFadeBuilder(Widget topChild, Key topChildKey, Widget /// /// * [AnimatedSize], the lower-level widget which [AnimatedCrossFade] uses to /// automatically change size. +/// * [AnimatedChildSwitcher], which switches out a child for a new one with a +/// customizable transition. class AnimatedCrossFade extends StatefulWidget { /// Creates a cross-fade animation widget. /// diff --git a/packages/flutter/test/rendering/stack_test.dart b/packages/flutter/test/rendering/stack_test.dart index 2e73e44396..839a453d8f 100644 --- a/packages/flutter/test/rendering/stack_test.dart +++ b/packages/flutter/test/rendering/stack_test.dart @@ -49,6 +49,17 @@ void main() { expect(green.size.height, equals(100.0)); }); + test('Stack can layout with no children', () { + final RenderBox stack = new RenderStack( + textDirection: TextDirection.ltr, + children: [], + ); + + layout(stack, constraints: new BoxConstraints.tight(const Size(100.0, 100.0))); + + expect(stack.size.width, equals(100.0)); + expect(stack.size.height, equals(100.0)); + }); group('RenderIndexedStack', () { test('visitChildrenForSemantics only visits displayed child', () { diff --git a/packages/flutter/test/widgets/animated_child_switcher_test.dart b/packages/flutter/test/widgets/animated_child_switcher_test.dart index 871204d5a7..b14436f50f 100644 --- a/packages/flutter/test/widgets/animated_child_switcher_test.dart +++ b/packages/flutter/test/widgets/animated_child_switcher_test.dart @@ -7,14 +7,63 @@ import 'package:flutter_test/flutter_test.dart'; void main() { testWidgets('AnimatedChildSwitcher fades in a new child.', (WidgetTester tester) async { + final UniqueKey containerOne = new UniqueKey(); + final UniqueKey containerTwo = new UniqueKey(); + final UniqueKey containerThree = new UniqueKey(); + await tester.pumpWidget( + new AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: new Container(key: containerOne, color: const Color(0x00000000)), + switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, + ), + ); + + FadeTransition transition = tester.firstWidget(find.byType(FadeTransition)); + expect(transition.opacity.value, equals(1.0)); + + await tester.pumpWidget( + new AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: new Container(key: containerTwo, color: const Color(0xff000000)), + switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, + ), + ); + + await tester.pump(const Duration(milliseconds: 50)); + transition = tester.firstWidget(find.byType(FadeTransition)); + expect(transition.opacity.value, equals(0.5)); + + await tester.pumpWidget( + new AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: new Container(key: containerThree, color: const Color(0xffff0000)), + switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, + ), + ); + + await tester.pump(const Duration(milliseconds: 10)); + transition = tester.widget(find.byType(FadeTransition).at(0)); + expect(transition.opacity.value, closeTo(0.4, 0.01)); + transition = tester.widget(find.byType(FadeTransition).at(1)); + expect(transition.opacity.value, closeTo(0.4, 0.01)); + transition = tester.widget(find.byType(FadeTransition).at(2)); + expect(transition.opacity.value, closeTo(0.1, 0.01)); + await tester.pumpAndSettle(); + }); + + testWidgets("AnimatedChildSwitcher doesn't transition in a new child of the same type.", (WidgetTester tester) async { await tester.pumpWidget( new AnimatedChildSwitcher( duration: const Duration(milliseconds: 100), child: new Container(color: const Color(0x00000000)), switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, ), ); - // First one just appears. + FadeTransition transition = tester.firstWidget(find.byType(FadeTransition)); expect(transition.opacity.value, equals(1.0)); @@ -23,12 +72,80 @@ void main() { duration: const Duration(milliseconds: 100), child: new Container(color: const Color(0xff000000)), switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, ), ); - // Second one cross-fades with the first. + + await tester.pump(const Duration(milliseconds: 50)); + transition = tester.widget(find.byType(FadeTransition)); + expect(transition.opacity.value, equals(1.0)); + await tester.pumpAndSettle(); + }); + + testWidgets('AnimatedChildSwitcher handles null children.', (WidgetTester tester) async { + await tester.pumpWidget( + const AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: null, + switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, + ), + ); + + expect(find.byType(FadeTransition), findsNothing); + + await tester.pumpWidget( + new AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: new Container(color: const Color(0xff000000)), + switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, + ), + ); + + await tester.pump(const Duration(milliseconds: 50)); + FadeTransition transition = tester.firstWidget(find.byType(FadeTransition)); + expect(transition.opacity.value, equals(0.5)); + await tester.pumpAndSettle(); + + await tester.pumpWidget( + new AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: new Container(color: const Color(0x00000000)), + switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, + ), + ); + + transition = tester.firstWidget(find.byType(FadeTransition)); + expect(transition.opacity.value, equals(1.0)); + + await tester.pumpWidget( + const AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: null, + switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, + ), + ); + await tester.pump(const Duration(milliseconds: 50)); transition = tester.firstWidget(find.byType(FadeTransition)); expect(transition.opacity.value, equals(0.5)); + + await tester.pumpWidget( + const AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: null, + switchInCurve: Curves.linear, + switchOutCurve: Curves.linear, + ), + ); + + await tester.pump(const Duration(milliseconds: 50)); + transition = tester.firstWidget(find.byType(FadeTransition)); + expect(transition.opacity.value, equals(0.0)); + await tester.pumpAndSettle(); }); @@ -44,4 +161,63 @@ void main() { await tester.pumpWidget(new Container(color: const Color(0xffff0000))); expect(await tester.pumpAndSettle(const Duration(milliseconds: 100)), equals(1)); }); + + testWidgets('AnimatedChildSwitcher uses custom layout.', (WidgetTester tester) async { + Widget newLayoutBuilder(List children) { + return new Column( + children: children, + ); + } + + await tester.pumpWidget( + new AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: new Container(color: const Color(0x00000000)), + switchInCurve: Curves.linear, + layoutBuilder: newLayoutBuilder, + ), + ); + + expect(find.byType(Column), findsOneWidget); + }); + + testWidgets('AnimatedChildSwitcher uses custom transitions.', (WidgetTester tester) async { + final List transitions = []; + Widget newLayoutBuilder(List children) { + transitions.clear(); + transitions.addAll(children); + return new Column( + children: children, + ); + } + + Widget newTransitionBuilder(Widget child, Animation animation) { + return new SizeTransition( + sizeFactor: animation, + child: child, + ); + } + + await tester.pumpWidget( + new Directionality( + textDirection: TextDirection.rtl, + child: new AnimatedChildSwitcher( + duration: const Duration(milliseconds: 100), + child: new Container(color: const Color(0x00000000)), + switchInCurve: Curves.linear, + layoutBuilder: newLayoutBuilder, + transitionBuilder: newTransitionBuilder, + ), + ), + ); + + expect(find.byType(Column), findsOneWidget); + for (Widget transition in transitions) { + expect(transition, const isInstanceOf()); + expect( + find.descendant(of: find.byWidget(transition), matching: find.byType(SizeTransition)), + findsOneWidget, + ); + } + }); }