From 695b48a7a8feba35a224e202ec1b3e95b17d7d37 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Wed, 21 Aug 2024 20:05:45 -0300 Subject: [PATCH 1/8] Fix a bug when animation duration is zero --- .../two_dimensional_scrollables/CHANGELOG.md | 4 + .../lib/src/tree_view/tree.dart | 10 ++ .../two_dimensional_scrollables/pubspec.yaml | 2 +- .../test/tree_view/tree_test.dart | 93 +++++++++++++++++++ 4 files changed, 108 insertions(+), 1 deletion(-) diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index 0bcac837523..34bcde5cae5 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.3.2 + +* Fixes a bug where the TreeView would not update correctly when the animation duration is zero. + ## 0.3.1 * Adds generics to the callbacks and builders of TreeView. diff --git a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart index ba6c4632c40..393c847cffe 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart @@ -886,6 +886,16 @@ class _TreeViewState extends State> if (widget.onNodeToggle != null) { widget.onNodeToggle!(node); } + + // If animation is disabled or the duration is zero, we skip the animation + // and immediately update the active nodes. This prevents the app from freezing + // due to the tree being incorrectly updated when the animation duration is zero. + // This is because, in this case, the node's children are no longer active. + if (widget.toggleAnimationStyle == AnimationStyle.noAnimation || widget.toggleAnimationStyle?.duration == Duration.zero) { + _unpackActiveNodes(); + return; + } + final AnimationController controller = _currentAnimationForParent[node]?.controller ?? AnimationController( diff --git a/packages/two_dimensional_scrollables/pubspec.yaml b/packages/two_dimensional_scrollables/pubspec.yaml index 45845c7afe0..6fbafb4d9a3 100644 --- a/packages/two_dimensional_scrollables/pubspec.yaml +++ b/packages/two_dimensional_scrollables/pubspec.yaml @@ -1,6 +1,6 @@ name: two_dimensional_scrollables description: Widgets that scroll using the two dimensional scrolling foundation. -version: 0.3.1 +version: 0.3.2 repository: https://github.com/flutter/packages/tree/main/packages/two_dimensional_scrollables issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+two_dimensional_scrollables%22+ diff --git a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart index a1790879fe9..a3604c61f03 100644 --- a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart +++ b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart @@ -718,6 +718,99 @@ void main() { expect(treeView.treeNodeBuilder, isA>()); expect(treeView.treeRowBuilder, isA>()); }); + + testWidgets('TreeSliverNode should close all children when collapsed when animation is completed', (WidgetTester tester) async { + final TreeViewController controller = TreeViewController(); + final List> tree = >[ + TreeViewNode('First'), + TreeViewNode( + 'Second', + children: >[ + TreeViewNode( + 'alpha', + children: >[ + TreeViewNode('uno'), + TreeViewNode('dos'), + TreeViewNode('tres'), + ], + ), + TreeViewNode('beta'), + TreeViewNode('kappa'), + ], + ), + TreeViewNode( + 'Third', + expanded: true, + children: >[ + TreeViewNode('gamma'), + TreeViewNode('delta'), + TreeViewNode('epsilon'), + ], + ), + TreeViewNode('Fourth'), + ]; + + await tester.pumpWidget(MaterialApp( + home: TreeView( + tree: tree, + controller: controller, + toggleAnimationStyle: AnimationStyle( + curve: Curves.easeInOut, + duration: Duration.zero, + ), + treeNodeBuilder: ( + BuildContext context, + TreeViewNode node, + AnimationStyle animationStyle, + ) { + final Widget child = GestureDetector( + behavior: HitTestBehavior.translucent, + onTap: () => controller.toggleNode(node), + child: TreeView.defaultTreeNodeBuilder( + context, + node, + animationStyle, + ), + ); + + return child; + }, + ), + )); + + expect(find.text('First'), findsOneWidget); + expect(find.text('Second'), findsOneWidget); + expect(find.text('Third'), findsOneWidget); + expect(find.text('Fourth'), findsOneWidget); + expect(find.text('alpha'), findsNothing); + expect(find.text('beta'), findsNothing); + expect(find.text('kappa'), findsNothing); + expect(find.text('gamma'), findsOneWidget); + expect(find.text('delta'), findsOneWidget); + expect(find.text('epsilon'), findsOneWidget); + expect(find.text('uno'), findsNothing); + expect(find.text('dos'), findsNothing); + expect(find.text('tres'), findsNothing); + + await tester.tap(find.text('Second')); + await tester.pumpAndSettle(); + + expect(find.text('alpha'), findsOneWidget); + + await tester.tap(find.text('alpha')); + await tester.pumpAndSettle(); + + expect(find.text('uno'), findsOneWidget); + expect(find.text('dos'), findsOneWidget); + expect(find.text('tres'), findsOneWidget); + + await tester.tap(find.text('alpha')); + await tester.pumpAndSettle(); + + expect(find.text('uno'), findsNothing); + expect(find.text('dos'), findsNothing); + expect(find.text('tres'), findsNothing); + }); }); group('TreeViewport', () { From f991bec97a2d3c065e53d7a457477215a4173c8a Mon Sep 17 00:00:00 2001 From: Mairramer Date: Thu, 22 Aug 2024 08:36:10 -0300 Subject: [PATCH 2/8] fix format --- .../lib/src/tree_view/tree.dart | 3 +- .../test/tree_view/tree_test.dart | 50 ++++++++++--------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart index 393c847cffe..c1b52acf3e6 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart @@ -891,7 +891,8 @@ class _TreeViewState extends State> // and immediately update the active nodes. This prevents the app from freezing // due to the tree being incorrectly updated when the animation duration is zero. // This is because, in this case, the node's children are no longer active. - if (widget.toggleAnimationStyle == AnimationStyle.noAnimation || widget.toggleAnimationStyle?.duration == Duration.zero) { + if (widget.toggleAnimationStyle == AnimationStyle.noAnimation || + widget.toggleAnimationStyle?.duration == Duration.zero) { _unpackActiveNodes(); return; } diff --git a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart index a3604c61f03..32edda20a6f 100644 --- a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart +++ b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart @@ -719,7 +719,9 @@ void main() { expect(treeView.treeRowBuilder, isA>()); }); - testWidgets('TreeSliverNode should close all children when collapsed when animation is completed', (WidgetTester tester) async { + testWidgets( + 'TreeSliverNode should close all children when collapsed when animation is completed', + (WidgetTester tester) async { final TreeViewController controller = TreeViewController(); final List> tree = >[ TreeViewNode('First'), @@ -752,31 +754,31 @@ void main() { await tester.pumpWidget(MaterialApp( home: TreeView( - tree: tree, - controller: controller, - toggleAnimationStyle: AnimationStyle( - curve: Curves.easeInOut, - duration: Duration.zero, + tree: tree, + controller: controller, + toggleAnimationStyle: AnimationStyle( + curve: Curves.easeInOut, + duration: Duration.zero, + ), + treeNodeBuilder: ( + BuildContext context, + TreeViewNode node, + AnimationStyle animationStyle, + ) { + final Widget child = GestureDetector( + behavior: HitTestBehavior.translucent, + onTap: () => controller.toggleNode(node), + child: TreeView.defaultTreeNodeBuilder( + context, + node, + animationStyle, ), - treeNodeBuilder: ( - BuildContext context, - TreeViewNode node, - AnimationStyle animationStyle, - ) { - final Widget child = GestureDetector( - behavior: HitTestBehavior.translucent, - onTap: () => controller.toggleNode(node), - child: TreeView.defaultTreeNodeBuilder( - context, - node, - animationStyle, - ), - ); + ); - return child; - }, - ), - )); + return child; + }, + ), + )); expect(find.text('First'), findsOneWidget); expect(find.text('Second'), findsOneWidget); From d77fcfde5ab48c47d1212c8fea14f067ff36a0a1 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Thu, 22 Aug 2024 16:05:13 -0300 Subject: [PATCH 3/8] fix --- .../two_dimensional_scrollables/test/tree_view/tree_test.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart index 32edda20a6f..151006722a7 100644 --- a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart +++ b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart @@ -719,8 +719,7 @@ void main() { expect(treeView.treeRowBuilder, isA>()); }); - testWidgets( - 'TreeSliverNode should close all children when collapsed when animation is completed', + testWidgets('TreeViewNode should expand/collapse correctly when the animation duration is set to zero.', (WidgetTester tester) async { final TreeViewController controller = TreeViewController(); final List> tree = >[ From 30b6a0e89a53fe522d8ded4e84838fda8888403e Mon Sep 17 00:00:00 2001 From: Mairramer Date: Thu, 22 Aug 2024 16:14:31 -0300 Subject: [PATCH 4/8] fix format --- .../two_dimensional_scrollables/test/tree_view/tree_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart index 151006722a7..7047af4fa05 100644 --- a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart +++ b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart @@ -719,7 +719,8 @@ void main() { expect(treeView.treeRowBuilder, isA>()); }); - testWidgets('TreeViewNode should expand/collapse correctly when the animation duration is set to zero.', + testWidgets( + 'TreeViewNode should expand/collapse correctly when the animation duration is set to zero.', (WidgetTester tester) async { final TreeViewController controller = TreeViewController(); final List> tree = >[ From be87d07d9ab5ea281ebfdffeec1cd63a2dfd5257 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Wed, 28 Aug 2024 17:46:31 -0300 Subject: [PATCH 5/8] added comment --- .../two_dimensional_scrollables/test/tree_view/tree_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart index 7047af4fa05..6177ce33707 100644 --- a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart +++ b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart @@ -722,6 +722,7 @@ void main() { testWidgets( 'TreeViewNode should expand/collapse correctly when the animation duration is set to zero.', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/154292 final TreeViewController controller = TreeViewController(); final List> tree = >[ TreeViewNode('First'), From ba41a6cae9490f20338a26075274f27b63f3c085 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Fri, 30 Aug 2024 08:47:45 -0300 Subject: [PATCH 6/8] fix --- .../lib/src/tree_view/tree.dart | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart index c1b52acf3e6..45715468e41 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart @@ -887,12 +887,14 @@ class _TreeViewState extends State> widget.onNodeToggle!(node); } - // If animation is disabled or the duration is zero, we skip the animation - // and immediately update the active nodes. This prevents the app from freezing - // due to the tree being incorrectly updated when the animation duration is zero. - // This is because, in this case, the node's children are no longer active. + // If animation is disabled or duration is zero, skip the animation + // and update the active nodes immediately. This ensures the tree + // is updated correctly when the node's children are no longer active. if (widget.toggleAnimationStyle == AnimationStyle.noAnimation || - widget.toggleAnimationStyle?.duration == Duration.zero) { + (node._expanded && + widget.toggleAnimationStyle?.duration == Duration.zero) || + (!node._expanded && + widget.toggleAnimationStyle?.reverseDuration == Duration.zero)) { _unpackActiveNodes(); return; } From 39877ef49103a420a54262e1fbe8a5fbf3a1fdf8 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Fri, 30 Aug 2024 08:50:33 -0300 Subject: [PATCH 7/8] fix --- .../two_dimensional_scrollables/lib/src/tree_view/tree.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart index 45715468e41..9392c9f3f7c 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart @@ -891,10 +891,7 @@ class _TreeViewState extends State> // and update the active nodes immediately. This ensures the tree // is updated correctly when the node's children are no longer active. if (widget.toggleAnimationStyle == AnimationStyle.noAnimation || - (node._expanded && - widget.toggleAnimationStyle?.duration == Duration.zero) || - (!node._expanded && - widget.toggleAnimationStyle?.reverseDuration == Duration.zero)) { + widget.toggleAnimationStyle?.duration == Duration.zero) { _unpackActiveNodes(); return; } From a3fbbd5d72afeb1b9a9eaa2f60e497e32be95f50 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Wed, 11 Sep 2024 17:52:54 -0300 Subject: [PATCH 8/8] improve --- .../two_dimensional_scrollables/lib/src/tree_view/tree.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart index 9392c9f3f7c..a3ba1bd218f 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart @@ -890,8 +890,7 @@ class _TreeViewState extends State> // If animation is disabled or duration is zero, skip the animation // and update the active nodes immediately. This ensures the tree // is updated correctly when the node's children are no longer active. - if (widget.toggleAnimationStyle == AnimationStyle.noAnimation || - widget.toggleAnimationStyle?.duration == Duration.zero) { + if (widget.toggleAnimationStyle?.duration == Duration.zero) { _unpackActiveNodes(); return; }