diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index 3eb9f1d569c0..c2acc0f460e6 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.5.5 + +* Fixes `TableView` laying out and painting trailing pinned rows and columns twice, which could throw when the table was scrolled to the end. + ## 0.5.4 * Fixes memory leaks. diff --git a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart index 94b104aec8b5..7e8fc05daa35 100644 --- a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart +++ b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart @@ -1087,7 +1087,9 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { (span) => !span.isPinned && span.trailingOffset >= _targetTrailingColumnPixel, ); if (_firstNonPinnedColumn != null) { - _lastNonPinnedColumn ??= _columnMetrics.length - 1; + // Trailing pinned columns are laid out and painted separately, so the + // range of regular columns must never extend into them. + _lastNonPinnedColumn ??= _lastRegularColumnIndex ?? _columnMetrics.length - 1; } if (_rowMetrics.isNotEmpty) { @@ -1116,7 +1118,9 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { (span) => !span.isPinned && span.trailingOffset >= _targetTrailingRowPixel, ); if (_firstNonPinnedRow != null) { - _lastNonPinnedRow ??= _rowMetrics.length - 1; + // Trailing pinned rows are laid out and painted separately, so the range + // of regular rows must never extend into them. + _lastNonPinnedRow ??= _lastRegularRowIndex ?? _rowMetrics.length - 1; } } diff --git a/packages/two_dimensional_scrollables/pubspec.yaml b/packages/two_dimensional_scrollables/pubspec.yaml index d112e4be1113..92ce2815d9ad 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.5.4 +version: 0.5.5 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/table_view/table_test.dart b/packages/two_dimensional_scrollables/test/table_view/table_test.dart index 8a507699b96a..71208a0687b6 100644 --- a/packages/two_dimensional_scrollables/test/table_view/table_test.dart +++ b/packages/two_dimensional_scrollables/test/table_view/table_test.dart @@ -39,6 +39,20 @@ TableSpan getMouseTrackingSpan( ); } +// Counts how many times a span's decoration is painted, keyed by span index. +class CountingSpanDecoration extends TableSpanDecoration { + const CountingSpanDecoration({required this.index, required this.paintCounts}); + + final int index; + final Map paintCounts; + + @override + void paint(TableSpanDecorationPaintDetails details) { + paintCounts.update(index, (int count) => count + 1, ifAbsent: () => 1); + super.paint(details); + } +} + void main() { group('TableView.builder', () { testWidgets('creates correct delegate', (WidgetTester tester) async { @@ -4008,6 +4022,103 @@ void main() { expect(tester.getRect(find.text('R9 C9')).top, 300); }); + // Regression test for https://github.com/flutter/flutter/issues/185842. + testWidgets('Trailing pinned columns are excluded from the non-pinned column range', ( + WidgetTester tester, + ) async { + final horizontalController = ScrollController(); + addTearDown(horizontalController.dispose); + final paintCounts = {}; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 400, + width: 400, + child: TableView.builder( + columnCount: 20, + rowCount: 4, + trailingPinnedColumnCount: 1, + horizontalDetails: ScrollableDetails.horizontal(controller: horizontalController), + columnBuilder: (int index) => TableSpan( + extent: const FixedTableSpanExtent(100), + backgroundDecoration: CountingSpanDecoration( + index: index, + paintCounts: paintCounts, + ), + ), + rowBuilder: (int index) => const TableSpan(extent: FixedTableSpanExtent(100)), + cellBuilder: (BuildContext context, TableVicinity vicinity) { + return TableViewCell(child: Text('R${vicinity.row} C${vicinity.column}')); + }, + ), + ), + ), + ), + ); + + // Once scrolled to the end, no regular column reaches the trailing edge of + // the layout target. That is where the range of non-pinned columns used to + // fall back to the last column of the table, which is a trailing pinned + // one, making it part of both the non-pinned and the trailing pinned + // region. + paintCounts.clear(); + horizontalController.jumpTo(horizontalController.position.maxScrollExtent); + await tester.pump(); + + expect(tester.takeException(), isNull); + // The trailing pinned column is decorated once, by the trailing pinned + // region only. + expect(paintCounts[19], 1); + expect(find.text('R0 C19'), findsOneWidget); + }); + + // Regression test for https://github.com/flutter/flutter/issues/185842. + testWidgets('Trailing pinned rows are excluded from the non-pinned row range', ( + WidgetTester tester, + ) async { + final verticalController = ScrollController(); + addTearDown(verticalController.dispose); + final paintCounts = {}; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 400, + width: 400, + child: TableView.builder( + columnCount: 4, + rowCount: 20, + trailingPinnedRowCount: 1, + verticalDetails: ScrollableDetails.vertical(controller: verticalController), + columnBuilder: (int index) => const TableSpan(extent: FixedTableSpanExtent(100)), + rowBuilder: (int index) => TableSpan( + extent: const FixedTableSpanExtent(100), + backgroundDecoration: CountingSpanDecoration( + index: index, + paintCounts: paintCounts, + ), + ), + cellBuilder: (BuildContext context, TableVicinity vicinity) { + return TableViewCell(child: Text('R${vicinity.row} C${vicinity.column}')); + }, + ), + ), + ), + ), + ); + + paintCounts.clear(); + verticalController.jumpTo(verticalController.position.maxScrollExtent); + await tester.pump(); + + expect(tester.takeException(), isNull); + expect(paintCounts[19], 1); + expect(find.text('R19 C0'), findsOneWidget); + }); + testWidgets('Intersections of leading and trailing pinned', (WidgetTester tester) async { const span = TableSpan(extent: FixedTableSpanExtent(100)); await tester.pumpWidget(