From 12de8cea52c86ad69794d7da0089d5c34cd2004a Mon Sep 17 00:00:00 2001 From: Mairramer Date: Wed, 21 Aug 2024 19:56:38 -0300 Subject: [PATCH 1/9] fix unpack to work correctly. --- .../two_dimensional_scrollables/CHANGELOG.md | 4 + .../lib/src/tree_view/tree.dart | 12 ++- .../two_dimensional_scrollables/pubspec.yaml | 2 +- .../test/tree_view/tree_test.dart | 93 +++++++++++++++++++ 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index 0bcac837523c..ae9a57adc1e6 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.3.2 + +* Fixes an issue where collapsing nodes in the TreeView didn't work correctly, + ## 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 ba6c4632c408..1138e440fb25 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart @@ -902,6 +902,14 @@ class _TreeViewState extends State> _currentAnimationForParent[node]!.controller.dispose(); _currentAnimationForParent.remove(node); _updateActiveAnimations(); + // If the node is collapsing, we need to rebuild the active nodes + // to reflect the new state. + // This is because the node's children are no longer active. + if (!node._expanded) { + setState(() { + _unpackActiveNodes(); + }); + } case AnimationStatus.forward: case AnimationStatus.reverse: } @@ -940,9 +948,7 @@ class _TreeViewState extends State> controller.forward(); case false: // Collapsing - controller.reverse().then((_) { - _unpackActiveNodes(); - }); + controller.reverse(); } }); } diff --git a/packages/two_dimensional_scrollables/pubspec.yaml b/packages/two_dimensional_scrollables/pubspec.yaml index 45845c7afe09..6fbafb4d9a3d 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 a1790879fe9a..75d9aa87d2fb 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: const Duration(milliseconds: 200), + ), + 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 96aabf8a5f5c6add9b81075264d23ac6646e5151 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Wed, 21 Aug 2024 20:08:39 -0300 Subject: [PATCH 2/9] fix identation --- .../two_dimensional_scrollables/lib/src/tree_view/tree.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1138e440fb25..fa0ad6caa814 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart @@ -907,7 +907,7 @@ class _TreeViewState extends State> // This is because the node's children are no longer active. if (!node._expanded) { setState(() { - _unpackActiveNodes(); + _unpackActiveNodes(); }); } case AnimationStatus.forward: From dac4bac6527ef55384dc1cc5af2c4ca0aa3a68dc Mon Sep 17 00:00:00 2001 From: Mairramer Date: Thu, 22 Aug 2024 08:37:51 -0300 Subject: [PATCH 3/9] fix format --- .../test/tree_view/tree_test.dart | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 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 75d9aa87d2fb..2f92d30e0527 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: const Duration(milliseconds: 200), + tree: tree, + controller: controller, + toggleAnimationStyle: AnimationStyle( + curve: Curves.easeInOut, + duration: const Duration(milliseconds: 200), + ), + 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 5afe75d64bd9ee1fe850ee8b3b463f491c6791f7 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Thu, 22 Aug 2024 16:01:22 -0300 Subject: [PATCH 4/9] 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 2f92d30e0527..bb6b8a72434f 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 close all child nodes when collapsed, once the animation is completed', (WidgetTester tester) async { final TreeViewController controller = TreeViewController(); final List> tree = >[ From 936c1cb2ba5e854c5350cd27b7ab2be7cd676d7a Mon Sep 17 00:00:00 2001 From: Mairramer Date: Thu, 22 Aug 2024 16:15:07 -0300 Subject: [PATCH 5/9] 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 bb6b8a72434f..57ca4d88b7ec 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 close all child nodes when collapsed, once the animation is completed', + testWidgets( + 'TreeViewNode should close all child nodes when collapsed, once the animation is completed', (WidgetTester tester) async { final TreeViewController controller = TreeViewController(); final List> tree = >[ From 24df710d41aa7cd2ae95fc40c383f31bc179d927 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Thu, 22 Aug 2024 17:22:18 -0300 Subject: [PATCH 6/9] fix --- packages/two_dimensional_scrollables/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index ae9a57adc1e6..3db168f64c55 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.3.2 -* Fixes an issue where collapsing nodes in the TreeView didn't work correctly, +* Fixes an issue where collapsing nodes in the TreeView didn't work correctly. ## 0.3.1 From 03e24ab661efee4a0a72a340bc67009d0e532a81 Mon Sep 17 00:00:00 2001 From: Mairramer Date: Mon, 23 Sep 2024 08:41:49 -0300 Subject: [PATCH 7/9] fix --- .../lib/src/tree_view/tree.dart | 10 +- .../test/tree_view/tree_test.dart | 95 ------------------- 2 files changed, 4 insertions(+), 101 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 fa0ad6caa814..d8d87a422a8a 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/tree.dart @@ -902,13 +902,11 @@ class _TreeViewState extends State> _currentAnimationForParent[node]!.controller.dispose(); _currentAnimationForParent.remove(node); _updateActiveAnimations(); - // If the node is collapsing, we need to rebuild the active nodes - // to reflect the new state. - // This is because the node's children are no longer active. + // If the node is collapsing, we need to unpack the active + // nodes to remove the ones that were removed from the tree. + // This is only necessary if the node is collapsing. if (!node._expanded) { - setState(() { - _unpackActiveNodes(); - }); + _unpackActiveNodes(); } case AnimationStatus.forward: case AnimationStatus.reverse: 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 57ca4d88b7ec..a1790879fe9a 100644 --- a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart +++ b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart @@ -718,101 +718,6 @@ void main() { expect(treeView.treeNodeBuilder, isA>()); expect(treeView.treeRowBuilder, isA>()); }); - - testWidgets( - 'TreeViewNode should close all child nodes when collapsed, once the 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: const Duration(milliseconds: 200), - ), - 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 6644a03dce9dbdf8cf085caa657d605de3ea68cb Mon Sep 17 00:00:00 2001 From: Mairramer Date: Mon, 23 Sep 2024 08:43:25 -0300 Subject: [PATCH 8/9] fix --- packages/two_dimensional_scrollables/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/two_dimensional_scrollables/pubspec.yaml b/packages/two_dimensional_scrollables/pubspec.yaml index b2ab81b0ae13..3cd575a8c76a 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.2 +version: 0.3.3 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+ From 50dfa6c815762bed049a9d3b9d8b229cb48b41ff Mon Sep 17 00:00:00 2001 From: Mairramer Date: Sat, 28 Sep 2024 16:10:41 -0300 Subject: [PATCH 9/9] added test --- .../test/tree_view/tree_test.dart | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) 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 6177ce33707b..5e536402b85a 100644 --- a/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart +++ b/packages/two_dimensional_scrollables/test/tree_view/tree_test.dart @@ -814,6 +814,74 @@ void main() { expect(find.text('dos'), findsNothing); expect(find.text('tres'), findsNothing); }); + + testWidgets( + 'TreeViewNode should close all child nodes when collapsed, once the animation is completed', + (WidgetTester tester) async { + final TreeViewController controller = TreeViewController(); + final List> tree = >[ + TreeViewNode( + 'First', + expanded: true, + children: >[ + TreeViewNode( + 'alpha', + expanded: true, + children: >[ + TreeViewNode('uno'), + TreeViewNode('dos'), + TreeViewNode('tres'), + ], + ), + TreeViewNode('beta'), + TreeViewNode('kappa'), + ], + ), + ]; + + await tester.pumpWidget(MaterialApp( + home: TreeView( + tree: tree, + controller: controller, + toggleAnimationStyle: AnimationStyle( + curve: Curves.easeInOut, + duration: const Duration(milliseconds: 200), + ), + 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('alpha'), findsOneWidget); + expect(find.text('uno'), findsOneWidget); + expect(find.text('dos'), findsOneWidget); + expect(find.text('tres'), findsOneWidget); + + // Using runAsync to handle collapse and animations properly. + await tester.runAsync(() async { + 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', () {