From 2484e28fa098d84fe0a8554d219387126050d60c Mon Sep 17 00:00:00 2001 From: Jim Gerth Date: Mon, 27 Feb 2023 10:10:33 +0100 Subject: [PATCH 1/3] Revert "Fixed AnimatedSwitcher chain produced duplicates" Revert the changes introduced by #107476, as they do not completely fix the issue of duplicate keys in the AnimatedSwitcher switcher as demonstrated by #121336, and introduce behavior that deviates from the AnimatedSwitcher's documentation. Specifically, children that are transitioning out are immediately removed from the widget tree, if another child with the same key is added, while the AnimatedSwitcher is clearly supposed to support multiple children with the same key transitioning in and out at the same time according to its documentation. --- .../lib/src/widgets/animated_switcher.dart | 3 +-- .../test/widgets/animated_switcher_test.dart | 19 ------------------- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/packages/flutter/lib/src/widgets/animated_switcher.dart b/packages/flutter/lib/src/widgets/animated_switcher.dart index e2d6828a6b376d..7057417429bc40 100644 --- a/packages/flutter/lib/src/widgets/animated_switcher.dart +++ b/packages/flutter/lib/src/widgets/animated_switcher.dart @@ -216,7 +216,6 @@ class AnimatedSwitcher extends StatefulWidget { /// This is an [AnimatedSwitcherTransitionBuilder] function. static Widget defaultTransitionBuilder(Widget child, Animation animation) { return FadeTransition( - key: ValueKey(child.key), opacity: animation, child: child, ); @@ -389,6 +388,6 @@ class _AnimatedSwitcherState extends State with TickerProvider @override Widget build(BuildContext context) { _rebuildOutgoingWidgetsIfNeeded(); - return widget.layoutBuilder(_currentEntry?.transition, _outgoingWidgets!.where((Widget outgoing) => outgoing.key != _currentEntry?.transition.key).toSet().toList()); + return widget.layoutBuilder(_currentEntry?.transition, _outgoingWidgets!); } } diff --git a/packages/flutter/test/widgets/animated_switcher_test.dart b/packages/flutter/test/widgets/animated_switcher_test.dart index 0d914cb73ed9c4..335ddd6083337f 100644 --- a/packages/flutter/test/widgets/animated_switcher_test.dart +++ b/packages/flutter/test/widgets/animated_switcher_test.dart @@ -415,25 +415,6 @@ void main() { ); } }); - - testWidgets('AnimatedSwitcher does not duplicate animations if the same child is entered twice.', (WidgetTester tester) async { - Future pumpChild(Widget child) async { - return tester.pumpWidget( - Directionality( - textDirection: TextDirection.ltr, - child: AnimatedSwitcher( - duration: const Duration(milliseconds: 1000), - child: child, - ), - ), - ); - } - await pumpChild(const Text('1', key: Key('1'))); - await pumpChild(const Text('2', key: Key('2'))); - await pumpChild(const Text('1', key: Key('1'))); - await tester.pump(const Duration(milliseconds: 1000)); - expect(find.text('1'), findsOneWidget); - }); } class StatefulTest extends StatefulWidget { From 3e8d9ca290fe258acd4b5bd7aa5c8b898bf73b83 Mon Sep 17 00:00:00 2001 From: Jim Gerth Date: Fri, 24 Feb 2023 16:37:25 +0100 Subject: [PATCH 2/3] Use AnimatedSwitcher's _childNumber as Key in layoutBuilder's Stack Previously KeyedSubtree.wrap gave children of the AnimatedSwitcher their own key as a key, if available, and their _childNumber as a key, if not. This could lead to multiple children having the same key in the defaultLayoutBuilder's Stack and thus a duplicate key exception being thrown, though. This is fixed by always using the uniquely identifiable _childNumber as a key for the children of the AnimatedSwitcher. This does not impair the functionality of having children of the same type be differentiated and transitioned between by giving them different keys, as the children's original key is still used to determine if a transition should take place or not. --- packages/flutter/lib/src/widgets/animated_switcher.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/flutter/lib/src/widgets/animated_switcher.dart b/packages/flutter/lib/src/widgets/animated_switcher.dart index 7057417429bc40..7dce6d9320faa7 100644 --- a/packages/flutter/lib/src/widgets/animated_switcher.dart +++ b/packages/flutter/lib/src/widgets/animated_switcher.dart @@ -337,7 +337,10 @@ class _AnimatedSwitcherState extends State with TickerProvider }) { final _ChildEntry entry = _ChildEntry( widgetChild: child, - transition: KeyedSubtree.wrap(builder(child, animation), _childNumber), + transition: KeyedSubtree( + key: ValueKey(_childNumber), + child: builder(child, animation), + ), animation: animation, controller: controller, ); From 8ac116e43e45e69e47bd636689dee75f0c36b468 Mon Sep 17 00:00:00 2001 From: Jim Gerth Date: Mon, 27 Feb 2023 11:11:31 +0100 Subject: [PATCH 3/3] Test for AnimatedSwitcher handling multiple children with the same key Add a test for the AnimatedSwitcher, that test if multiple children with the same key can be added and are properly transitioned in and out. --- .../test/widgets/animated_switcher_test.dart | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/flutter/test/widgets/animated_switcher_test.dart b/packages/flutter/test/widgets/animated_switcher_test.dart index 335ddd6083337f..4dcacebb750ef3 100644 --- a/packages/flutter/test/widgets/animated_switcher_test.dart +++ b/packages/flutter/test/widgets/animated_switcher_test.dart @@ -415,6 +415,46 @@ void main() { ); } }); + + testWidgets('AnimatedSwitcher can handle multiple children with the same key.', (WidgetTester tester) async { + final UniqueKey containerA = UniqueKey(); + final UniqueKey containerB = UniqueKey(); + + // Pump an AnimatedSwitcher with a child container with the given key. + Future pump(Key key) async { + return tester.pumpWidget( + AnimatedSwitcher( + duration: const Duration(milliseconds: 1000), + child: Container(key: key), + ), + ); + } + + // Pump four widgets with the two keys A and B in alternating order. + await pump(containerA); + await tester.pump(const Duration(milliseconds: 1000)); + await pump(containerB); + await tester.pump(const Duration(milliseconds: 500)); + await pump(containerA); + await tester.pump(const Duration(milliseconds: 250)); + await pump(containerB); + await tester.pump(const Duration(milliseconds: 125)); + + // All four widgets should still be animating in (the one pumped last) or + // out (the other ones), and thus have an associated FadeTransition each. + expect(find.byType(FadeTransition), findsNWidgets(4)); + final Iterable transitions = tester.widgetList( + find.byType(FadeTransition), + ); + + // The exponentially decaying timing used in pumping the widgets should have + // lined up all of the FadeTransitions' values to be the same. + for (final FadeTransition transition in transitions) { + expect(transition.opacity.value, moreOrLessEquals(0.125, epsilon: 0.001)); + } + + await tester.pumpAndSettle(); + }); } class StatefulTest extends StatefulWidget {