Skip to content

Commit 4380aee

Browse files
PiinksPiinks
committed
Fix crash due to empty or fully collapsed tree
1 parent c3360ac commit 4380aee

4 files changed

Lines changed: 161 additions & 45 deletions

File tree

‎packages/two_dimensional_scrollables/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 0.5.2
2+
3+
* Fixes a crash in `TreeView` when it shrinks to 0 rows or the last node is collapsed.
4+
15
## 0.5.1
26

37
* Fixes an infinite loop of onExit/onEnter events when setState is called within onEnter in a TableSpan.

‎packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart‎

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -346,13 +346,13 @@ class RenderTreeViewport extends RenderTwoDimensionalViewport {
346346
);
347347
_horizontalOverflows = maxHorizontalExtent > 0.0;
348348

349-
final double verticalLeadingExtent = verticalOffset.pixels;
350-
final double verticalTrailingExtent =
351-
_rowMetrics[_lastRow!]!.trailingOffset - viewportDimension.height;
352-
final double maxVerticalExtent = math.max(
353-
0.0,
354-
math.max(verticalLeadingExtent, verticalTrailingExtent),
355-
);
349+
final double maxVerticalExtent = _rowMetrics.isEmpty
350+
? 0.0
351+
: math.max(
352+
0.0,
353+
_rowMetrics[_rowMetrics.length - 1]!.trailingOffset -
354+
viewportDimension.height,
355+
);
356356
_verticalOverflows = maxVerticalExtent > 0.0;
357357

358358
final bool acceptedDimension =
@@ -389,43 +389,40 @@ class RenderTreeViewport extends RenderTwoDimensionalViewport {
389389
}
390390
}
391391

392-
if (_firstRow == null) {
393-
assert(_lastRow == null);
394-
return;
395-
}
396-
assert(_firstRow != null && _lastRow != null);
397-
398-
_Span rowSpan;
399-
double rowOffset =
400-
-verticalOffset.pixels +
401-
_rowMetrics[_firstRow!]!.leadingOffset +
402-
_vAlignmentOffset;
403-
for (int row = _firstRow!; row <= _lastRow!; row++) {
404-
rowSpan = _rowMetrics[row]!;
405-
final double rowHeight = rowSpan.extent;
406-
if (_animationLeadingIndices.keys.contains(row)) {
407-
rowOffset -= rowSpan.animationOffset;
392+
if (_firstRow != null) {
393+
assert(_lastRow != null);
394+
_Span rowSpan;
395+
double rowOffset =
396+
-verticalOffset.pixels +
397+
_rowMetrics[_firstRow!]!.leadingOffset +
398+
_vAlignmentOffset;
399+
for (int row = _firstRow!; row <= _lastRow!; row++) {
400+
rowSpan = _rowMetrics[row]!;
401+
final double rowHeight = rowSpan.extent;
402+
if (_animationLeadingIndices.keys.contains(row)) {
403+
rowOffset -= rowSpan.animationOffset;
404+
}
405+
rowOffset += rowSpan.configuration.padding.leading;
406+
407+
final vicinity = TreeVicinity(depth: _rowDepths[row]!, row: row);
408+
final RenderBox child = buildOrObtainChildFor(vicinity)!;
409+
final TwoDimensionalViewportParentData parentData = parentDataOf(child);
410+
final childConstraints = BoxConstraints(
411+
minHeight: rowHeight,
412+
maxHeight: rowHeight,
413+
// Width is allowed to be unbounded.
414+
);
415+
child.layout(childConstraints, parentUsesSize: true);
416+
parentData.layoutOffset = Offset(
417+
(_rowDepths[row]! * indentation) - horizontalOffset.pixels,
418+
rowOffset,
419+
);
420+
rowOffset += rowHeight + rowSpan.configuration.padding.trailing;
421+
_furthestHorizontalExtent = math.max(
422+
parentData.layoutOffset!.dx + child.size.width,
423+
_furthestHorizontalExtent,
424+
);
408425
}
409-
rowOffset += rowSpan.configuration.padding.leading;
410-
411-
final vicinity = TreeVicinity(depth: _rowDepths[row]!, row: row);
412-
final RenderBox child = buildOrObtainChildFor(vicinity)!;
413-
final TwoDimensionalViewportParentData parentData = parentDataOf(child);
414-
final childConstraints = BoxConstraints(
415-
minHeight: rowHeight,
416-
maxHeight: rowHeight,
417-
// Width is allowed to be unbounded.
418-
);
419-
child.layout(childConstraints, parentUsesSize: true);
420-
parentData.layoutOffset = Offset(
421-
(_rowDepths[row]! * indentation) - horizontalOffset.pixels,
422-
rowOffset,
423-
);
424-
rowOffset += rowHeight + rowSpan.configuration.padding.trailing;
425-
_furthestHorizontalExtent = math.max(
426-
parentData.layoutOffset!.dx + child.size.width,
427-
_furthestHorizontalExtent,
428-
);
429426
}
430427
_updateScrollBounds();
431428
}

‎packages/two_dimensional_scrollables/pubspec.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
name: two_dimensional_scrollables
22
description: Widgets that scroll using the two dimensional scrolling foundation.
3-
version: 0.5.1
3+
version: 0.5.2
44
repository: https://github.com/flutter/packages/tree/main/packages/two_dimensional_scrollables
55
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+two_dimensional_scrollables%22+
66

‎packages/two_dimensional_scrollables/test/tree_view/render_tree_test.dart‎

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ void main() {
682682
await tester.pumpWidget(MaterialApp(home: treeView));
683683
await tester.pump();
684684
expect(verticalController.position.pixels, 0.0);
685-
expect(verticalController.position.maxScrollExtent, 600.0);
685+
expect(verticalController.position.maxScrollExtent, 2200.0);
686686

687687
bool rowNeedsPaint(String row) {
688688
return find.text(row).evaluate().first.renderObject!.debugNeedsPaint;
@@ -866,6 +866,121 @@ void main() {
866866
);
867867
});
868868
});
869+
870+
group('Scroll bounds', () {
871+
testWidgets(
872+
'shrinking to 0 rows updates scroll bounds and does not crash',
873+
(WidgetTester tester) async {
874+
var rows = 10;
875+
late StateSetter setState;
876+
final controller = ScrollController();
877+
await tester.pumpWidget(
878+
MaterialApp(
879+
home: Scaffold(
880+
body: SizedBox(
881+
height: 400,
882+
width: 400,
883+
child: StatefulBuilder(
884+
builder: (BuildContext context, StateSetter setter) {
885+
setState = setter;
886+
return TreeView<String>(
887+
verticalDetails: ScrollableDetails.vertical(
888+
controller: controller,
889+
),
890+
tree: List<TreeViewNode<String>>.generate(
891+
rows,
892+
(int index) => TreeViewNode<String>('Row $index'),
893+
),
894+
treeRowBuilder: (TreeViewNode<String> node) =>
895+
const TreeRow(extent: FixedTreeRowExtent(64.0)),
896+
treeNodeBuilder:
897+
(
898+
BuildContext context,
899+
TreeViewNode<String> node,
900+
AnimationStyle toggleAnimationStyle,
901+
) => Text(node.content),
902+
);
903+
},
904+
),
905+
),
906+
),
907+
),
908+
);
909+
910+
await tester.pump();
911+
final double oldMax = controller.position.maxScrollExtent;
912+
expect(oldMax, greaterThan(0));
913+
controller.jumpTo(oldMax);
914+
await tester.pump();
915+
expect(controller.offset, oldMax);
916+
917+
// Shrink to 0 rows.
918+
setState(() {
919+
rows = 0;
920+
});
921+
// This should not crash and should update scroll bounds.
922+
await tester.pump();
923+
924+
expect(controller.position.maxScrollExtent, 0.0);
925+
expect(controller.offset, 0.0);
926+
},
927+
);
928+
929+
testWidgets(
930+
'collapsing last node updates scroll bounds and does not crash',
931+
(WidgetTester tester) async {
932+
final treeController = TreeViewController();
933+
final scrollController = ScrollController();
934+
935+
final treeNodes = <TreeViewNode<String>>[
936+
TreeViewNode<String>(
937+
'Root',
938+
expanded: true,
939+
children: <TreeViewNode<String>>[TreeViewNode<String>('Child')],
940+
),
941+
];
942+
943+
await tester.pumpWidget(
944+
MaterialApp(
945+
home: Scaffold(
946+
body: SizedBox(
947+
height: 100,
948+
width: 400,
949+
child: TreeView<String>(
950+
controller: treeController,
951+
verticalDetails: ScrollableDetails.vertical(
952+
controller: scrollController,
953+
),
954+
tree: treeNodes,
955+
treeRowBuilder: (TreeViewNode<String> node) =>
956+
const TreeRow(extent: FixedTreeRowExtent(60.0)),
957+
treeNodeBuilder:
958+
(
959+
BuildContext context,
960+
TreeViewNode<String> node,
961+
AnimationStyle toggleAnimationStyle,
962+
) => Text(node.content),
963+
),
964+
),
965+
),
966+
),
967+
);
968+
969+
await tester.pump();
970+
// Root (60) + Child (60) = 120. Viewport is 100.
971+
expect(scrollController.position.maxScrollExtent, 20.0);
972+
scrollController.jumpTo(20.0);
973+
await tester.pump();
974+
975+
// Collapse Root. Now only Root (60) is visible.
976+
treeController.toggleNode(treeNodes[0]);
977+
await tester.pumpAndSettle();
978+
979+
expect(scrollController.position.maxScrollExtent, 0.0);
980+
expect(scrollController.offset, 0.0);
981+
},
982+
);
983+
});
869984
});
870985
}
871986

0 commit comments

Comments
 (0)