Review feedback

flutter · May 9, 2024 · 5a0194c · 5a0194c
1 parent ded7ae0
commit 5a0194c
Show file tree

Hide file tree

Showing 12 changed files with 315 additions and 225 deletions.
diff --git a/packages/two_dimensional_scrollables/example/lib/table_view/infinite_table.dart b/packages/two_dimensional_scrollables/example/lib/table_view/infinite_table.dart
@@ -5,7 +5,7 @@
 import 'package:flutter/material.dart';
 import 'package:two_dimensional_scrollables/two_dimensional_scrollables.dart';
 
-/// The class demonstrating and infinite number of rows and columns in
+/// The class demonstrating an infinite number of rows and columns in
 /// TableView.
 class InfiniteTableExample extends StatefulWidget {
   /// Creates a screen that demonstrates an infinite TableView widget.
@@ -74,13 +74,15 @@ class _InfiniteExampleState extends State<InfiniteTableExample> {
   }
 
   TableViewCell _buildCell(BuildContext context, TableVicinity vicinity) {
+    final Color boxColor =
+        switch ((vicinity.row.isEven, vicinity.column.isEven)) {
+      (true, false) || (false, true) => Colors.white,
+      (false, false) => Colors.indigo[100]!,
+      (true, true) => Colors.indigo[200]!
+    };
     return TableViewCell(
       child: ColoredBox(
-        color: vicinity.row.isEven && vicinity.column.isOdd
-            ? Colors.indigo[200]!
-            : (vicinity.row.isOdd && vicinity.column.isEven
-                ? Colors.indigo[100]!
-                : Colors.white),
+        color: boxColor,
         child: Center(
           child: Text('${vicinity.column}:${vicinity.row}'),
         ),

diff --git a/packages/two_dimensional_scrollables/example/lib/table_view/simple_table.dart b/packages/two_dimensional_scrollables/example/lib/table_view/simple_table.dart
@@ -23,6 +23,12 @@ class _TableExampleState extends State<TableExample> {
   late final ScrollController _verticalController = ScrollController();
   int _rowCount = 20;
 
+  @override
+  void dispose() {
+    _verticalController.dispose();
+    super.dispose();
+  }
+
   @override
   Widget build(BuildContext context) {
     return Scaffold(

diff --git a/packages/two_dimensional_scrollables/example/lib/tree_view/custom_tree.dart b/packages/two_dimensional_scrollables/example/lib/tree_view/custom_tree.dart
@@ -105,9 +105,9 @@ class CustomTreeExampleState extends State<CustomTreeExample> {
 
   Widget _treeNodeBuilder(
     BuildContext context,
-    TreeViewNode<dynamic> node, {
-    AnimationStyle? animationStyle,
-  }) {
+    TreeViewNode<Object?> node,
+    AnimationStyle toggleAnimationStyle,
+  ) {
     final bool isParentNode = node.children.isNotEmpty;
     final BorderSide border = BorderSide(
       width: 2,
@@ -188,13 +188,13 @@ class CustomTreeExampleState extends State<CustomTreeExample> {
               controller: _horizontalController,
             ),
             tree: _tree,
-            onNodeToggle: (TreeViewNode<dynamic> node) {
+            onNodeToggle: (TreeViewNode<Object?> node) {
               setState(() {
                 _selectedNode = node as TreeViewNode<String>;
               });
             },
             treeNodeBuilder: _treeNodeBuilder,
-            treeRowBuilder: (TreeViewNode<dynamic> node) {
+            treeRowBuilder: (TreeViewNode<Object?> node) {
               if (_selectedNode == (node as TreeViewNode<String>)) {
                 return TreeRow(
                   extent: FixedTreeRowExtent(
@@ -224,12 +224,18 @@ class CustomTreeExampleState extends State<CustomTreeExample> {
     );
   }
 
+  @override
+  void dispose() {
+    verticalController.dispose();
+    _horizontalController.dispose();
+    super.dispose();
+  }
+
   @override
   Widget build(BuildContext context) {
     final Size screenSize = MediaQuery.sizeOf(context);
     final List<Widget> selectedChildren = <Widget>[];
     if (_selectedNode != null) {
-      selectedChildren.clear();
       selectedChildren.addAll(<Widget>[
         const Spacer(),
         Icon(

diff --git a/packages/two_dimensional_scrollables/example/lib/tree_view/simple_tree.dart b/packages/two_dimensional_scrollables/example/lib/tree_view/simple_tree.dart
@@ -100,12 +100,12 @@ class TreeExampleState extends State<TreeExample> {
               controller: horizontalController,
             ),
             tree: _tree,
-            onNodeToggle: (TreeViewNode<dynamic> node) {
+            onNodeToggle: (TreeViewNode<Object?> node) {
               setState(() {
                 _selectedNode = node as TreeViewNode<String>;
               });
             },
-            treeRowBuilder: (TreeViewNode<dynamic> node) {
+            treeRowBuilder: (TreeViewNode<Object?> node) {
               if (_selectedNode == (node as TreeViewNode<String>)) {
                 return TreeView.defaultTreeRowBuilder(node).copyWith(
                   recognizerFactories: _getTapRecognizer(node),
@@ -126,6 +126,13 @@ class TreeExampleState extends State<TreeExample> {
     );
   }
 
+  @override
+  void dispose() {
+    _verticalController.dispose();
+    horizontalController.dispose();
+    super.dispose();
+  }
+
   @override
   Widget build(BuildContext context) {
     final Size screenSize = MediaQuery.sizeOf(context);

diff --git a/packages/two_dimensional_scrollables/example/test/tree_view/custom_tree_test.dart b/packages/two_dimensional_scrollables/example/test/tree_view/custom_tree_test.dart
@@ -18,7 +18,7 @@ void main() {
     expect(find.text('common'), findsOneWidget);
   });
 
-  testWidgets('Can scroll ', (WidgetTester tester) async {
+  testWidgets('Can scroll', (WidgetTester tester) async {
     await tester.pumpWidget(const MaterialApp(home: CustomTreeExample()));
     await tester.pumpAndSettle();
 

diff --git a/packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart b/packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart
@@ -99,7 +99,11 @@ class RenderTreeViewport extends RenderTwoDimensionalViewport {
       return;
     }
     _traversalOrder = value;
-    // We don't need to layout again. This is used when we visit children.
+    // Changing mainAxis will call markNeedsLayout.
+    mainAxis = switch (value) {
+      TreeViewTraversalOrder.depthFirst => Axis.vertical,
+      TreeViewTraversalOrder.breadthFirst => Axis.horizontal,
+    };
   }
 
   /// The number of pixels by which child nodes will be offset in the cross axis
@@ -174,7 +178,8 @@ class RenderTreeViewport extends RenderTwoDimensionalViewport {
         row = childAfter(row);
         continue;
       }
-      final Rect rowRect = parentData.paintOffset! & row.size;
+      final Rect rowRect = parentData.paintOffset! &
+          Size(viewportDimension.width, row.size.height);
       if (rowRect.contains(position)) {
         result.addWithPaintOffset(
           offset: parentData.paintOffset,