From 67470fc95e54e28a5e16f107277687a3e63987fa Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Thu, 22 Feb 2024 16:10:06 -0600 Subject: [PATCH] [two_dimensional_scrollables] Fix layout offset of merged pinned cells (#6178) Fixes https://github.com/flutter/flutter/issues/143526 This fixes the layout offset computation when there are merged cells within pinned rows and/or columns in TableView. --- .../two_dimensional_scrollables/CHANGELOG.md | 4 + .../lib/src/table_view/table.dart | 30 +- .../two_dimensional_scrollables/pubspec.yaml | 2 +- .../test/table_view/table_test.dart | 476 ++++++++++++++++++ 4 files changed, 509 insertions(+), 3 deletions(-) diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index 52ea3b087a7f..64d1a0a6a552 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.1.1 + +* Fixes a layout issue when pinned cells are merged. + ## 0.1.0 * [Breaking change] Adds support for merged cells in the TableView. 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 5d1575b1f5af..917881a61c29 100644 --- a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart +++ b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart @@ -801,7 +801,20 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { // | <--------- extent of merged cell ---------> | // Compute height and layout offset for merged rows. - mergedRowOffset = -verticalOffset.pixels + + final bool rowIsInPinnedColumn = _lastPinnedColumn != null && + vicinity.column <= _lastPinnedColumn!; + final bool rowIsPinned = + _lastPinnedRow != null && firstRow <= _lastPinnedRow!; + final double baseRowOffset = + switch ((rowIsInPinnedColumn, rowIsPinned)) { + // Both row and column are pinned at this cell, or just pinned row. + (true, true) || (false, true) => 0.0, + // Cell is within a pinned column + (true, false) => _pinnedRowsExtent - verticalOffset.pixels, + // Cell is within a pinned row, or no pinned portion. + (false, false) => -verticalOffset.pixels, + }; + mergedRowOffset = baseRowOffset + _rowMetrics[firstRow]!.leadingOffset + _rowMetrics[firstRow]!.configuration.padding.leading; mergedRowHeight = _rowMetrics[lastRow]!.trailingOffset - @@ -809,7 +822,20 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { _rowMetrics[lastRow]!.configuration.padding.trailing - _rowMetrics[firstRow]!.configuration.padding.leading; // Compute width and layout offset for merged columns. - mergedColumnOffset = -horizontalOffset.pixels + + final bool columnIsInPinnedRow = + _lastPinnedRow != null && vicinity.row <= _lastPinnedRow!; + final bool columnIsPinned = + _lastPinnedColumn != null && firstColumn <= _lastPinnedColumn!; + final double baseColumnOffset = + switch ((columnIsInPinnedRow, columnIsPinned)) { + // Both row and column are pinned at this cell, or just pinned column. + (true, true) || (false, true) => 0.0, + // Cell is within a pinned row. + (true, false) => _pinnedColumnsExtent - horizontalOffset.pixels, + // No pinned portion. + (false, false) => -horizontalOffset.pixels, + }; + mergedColumnOffset = baseColumnOffset + _columnMetrics[firstColumn]!.leadingOffset + _columnMetrics[firstColumn]!.configuration.padding.leading; mergedColumnWidth = _columnMetrics[lastColumn]!.trailingOffset - diff --git a/packages/two_dimensional_scrollables/pubspec.yaml b/packages/two_dimensional_scrollables/pubspec.yaml index e67277843dee..8d04d42f6455 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.1.0 +version: 0.1.1 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 7d59ad8aa573..c24d1927405e 100644 --- a/packages/two_dimensional_scrollables/test/table_view/table_test.dart +++ b/packages/two_dimensional_scrollables/test/table_view/table_test.dart @@ -1730,6 +1730,482 @@ void main() { SystemMouseCursors.basic, ); }); + + group('Merged pinned cells layout', () { + // Regression tests for https://github.com/flutter/flutter/issues/143526 + // These tests all use the same collection of merged pinned cells in a + // variety of combinations. + final Map bothMerged = + { + TableVicinity.zero: (start: 0, span: 2), + const TableVicinity(row: 1, column: 0): (start: 0, span: 2), + const TableVicinity(row: 0, column: 1): (start: 0, span: 2), + const TableVicinity(row: 1, column: 1): (start: 0, span: 2), + }; + + final Map rowMerged = + { + const TableVicinity(row: 2, column: 0): (start: 2, span: 2), + const TableVicinity(row: 3, column: 0): (start: 2, span: 2), + const TableVicinity(row: 4, column: 1): (start: 4, span: 3), + const TableVicinity(row: 5, column: 1): (start: 4, span: 3), + const TableVicinity(row: 6, column: 1): (start: 4, span: 3), + }; + + final Map columnMerged = + { + const TableVicinity(row: 0, column: 2): (start: 2, span: 2), + const TableVicinity(row: 0, column: 3): (start: 2, span: 2), + const TableVicinity(row: 1, column: 4): (start: 4, span: 3), + const TableVicinity(row: 1, column: 5): (start: 4, span: 3), + const TableVicinity(row: 1, column: 6): (start: 4, span: 3), + }; + const TableSpan span = TableSpan(extent: FixedTableSpanExtent(75)); + + testWidgets('Normal axes', (WidgetTester tester) async { + final ScrollController verticalController = ScrollController(); + final ScrollController horizontalController = ScrollController(); + final TableView tableView = TableView.builder( + verticalDetails: ScrollableDetails.vertical( + controller: verticalController, + ), + horizontalDetails: ScrollableDetails.horizontal( + controller: horizontalController, + ), + columnCount: 20, + rowCount: 20, + pinnedRowCount: 2, + pinnedColumnCount: 2, + columnBuilder: (_) => span, + rowBuilder: (_) => span, + cellBuilder: (_, TableVicinity vicinity) { + return TableViewCell( + columnMergeStart: + bothMerged[vicinity]?.start ?? columnMerged[vicinity]?.start, + columnMergeSpan: + bothMerged[vicinity]?.span ?? columnMerged[vicinity]?.span, + rowMergeStart: + bothMerged[vicinity]?.start ?? rowMerged[vicinity]?.start, + rowMergeSpan: + bothMerged[vicinity]?.span ?? rowMerged[vicinity]?.span, + child: Text( + 'R${bothMerged[vicinity]?.start ?? rowMerged[vicinity]?.start ?? vicinity.row}:' + 'C${bothMerged[vicinity]?.start ?? columnMerged[vicinity]?.start ?? vicinity.column}', + ), + ); + }, + ); + await tester.pumpWidget(MaterialApp(home: tableView)); + await tester.pumpAndSettle(); + + expect(verticalController.position.pixels, 0.0); + expect(horizontalController.position.pixels, 0.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(0.0, 0.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(150.0, 0.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(300.0, 75.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(0.0, 150.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(75.0, 300.0, 75.0, 225.0), + ); + + verticalController.jumpTo(10.0); + await tester.pumpAndSettle(); + expect(verticalController.position.pixels, 10.0); + expect(horizontalController.position.pixels, 0.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(0.0, 0.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(150.0, 0.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(300.0, 75.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(0.0, 140.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(75.0, 290.0, 75.0, 225.0), + ); + + horizontalController.jumpTo(10.0); + await tester.pumpAndSettle(); + expect(verticalController.position.pixels, 10.0); + expect(horizontalController.position.pixels, 10.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(0.0, 0.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(140.0, 0.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(290.0, 75.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(0.0, 140.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(75.0, 290.0, 75.0, 225.0), + ); + }); + + testWidgets('Vertical reversed', (WidgetTester tester) async { + final ScrollController verticalController = ScrollController(); + final ScrollController horizontalController = ScrollController(); + final TableView tableView = TableView.builder( + verticalDetails: ScrollableDetails.vertical( + reverse: true, + controller: verticalController, + ), + horizontalDetails: ScrollableDetails.horizontal( + controller: horizontalController, + ), + columnCount: 20, + rowCount: 20, + pinnedRowCount: 2, + pinnedColumnCount: 2, + columnBuilder: (_) => span, + rowBuilder: (_) => span, + cellBuilder: (_, TableVicinity vicinity) { + return TableViewCell( + columnMergeStart: + bothMerged[vicinity]?.start ?? columnMerged[vicinity]?.start, + columnMergeSpan: + bothMerged[vicinity]?.span ?? columnMerged[vicinity]?.span, + rowMergeStart: + bothMerged[vicinity]?.start ?? rowMerged[vicinity]?.start, + rowMergeSpan: + bothMerged[vicinity]?.span ?? rowMerged[vicinity]?.span, + child: Text( + 'R${bothMerged[vicinity]?.start ?? rowMerged[vicinity]?.start ?? vicinity.row}:' + 'C${bothMerged[vicinity]?.start ?? columnMerged[vicinity]?.start ?? vicinity.column}', + ), + ); + }, + ); + await tester.pumpWidget(MaterialApp(home: tableView)); + await tester.pumpAndSettle(); + + expect(verticalController.position.pixels, 0.0); + expect(horizontalController.position.pixels, 0.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(0.0, 450.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(150.0, 525.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(300.0, 450.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(0.0, 300.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(75.0, 75.0, 75.0, 225.0), + ); + + verticalController.jumpTo(10.0); + await tester.pumpAndSettle(); + expect(verticalController.position.pixels, 10.0); + expect(horizontalController.position.pixels, 0.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(0.0, 450.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(150.0, 525.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(300.0, 450.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(0.0, 310.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(75.0, 85.0, 75.0, 225.0), + ); + + horizontalController.jumpTo(10.0); + await tester.pumpAndSettle(); + expect(verticalController.position.pixels, 10.0); + expect(horizontalController.position.pixels, 10.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(0.0, 450.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(140.0, 525.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(290.0, 450.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(0.0, 310.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(75.0, 85.0, 75.0, 225.0), + ); + }); + + testWidgets('Horizontal reversed', (WidgetTester tester) async { + final ScrollController verticalController = ScrollController(); + final ScrollController horizontalController = ScrollController(); + final TableView tableView = TableView.builder( + verticalDetails: ScrollableDetails.vertical( + controller: verticalController, + ), + horizontalDetails: ScrollableDetails.horizontal( + reverse: true, + controller: horizontalController, + ), + columnCount: 20, + rowCount: 20, + pinnedRowCount: 2, + pinnedColumnCount: 2, + columnBuilder: (_) => span, + rowBuilder: (_) => span, + cellBuilder: (_, TableVicinity vicinity) { + return TableViewCell( + columnMergeStart: + bothMerged[vicinity]?.start ?? columnMerged[vicinity]?.start, + columnMergeSpan: + bothMerged[vicinity]?.span ?? columnMerged[vicinity]?.span, + rowMergeStart: + bothMerged[vicinity]?.start ?? rowMerged[vicinity]?.start, + rowMergeSpan: + bothMerged[vicinity]?.span ?? rowMerged[vicinity]?.span, + child: Text( + 'R${bothMerged[vicinity]?.start ?? rowMerged[vicinity]?.start ?? vicinity.row}:' + 'C${bothMerged[vicinity]?.start ?? columnMerged[vicinity]?.start ?? vicinity.column}', + ), + ); + }, + ); + await tester.pumpWidget(MaterialApp(home: tableView)); + await tester.pumpAndSettle(); + + expect(verticalController.position.pixels, 0.0); + expect(horizontalController.position.pixels, 0.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(650.0, 0.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(500.0, 0.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(275.0, 75.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(725.0, 150.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(650.0, 300.0, 75.0, 225.0), + ); + + verticalController.jumpTo(10.0); + await tester.pumpAndSettle(); + expect(verticalController.position.pixels, 10.0); + expect(horizontalController.position.pixels, 0.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(650.0, 0.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(500.0, 0.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(275.0, 75.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(725.0, 140.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(650.0, 290.0, 75.0, 225.0), + ); + + horizontalController.jumpTo(10.0); + await tester.pumpAndSettle(); + expect(verticalController.position.pixels, 10.0); + expect(horizontalController.position.pixels, 10.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(650.0, 0.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(510.0, 0.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(285.0, 75.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(725.0, 140.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(650.0, 290.0, 75.0, 225.0), + ); + }); + + testWidgets('Both reversed', (WidgetTester tester) async { + final ScrollController verticalController = ScrollController(); + final ScrollController horizontalController = ScrollController(); + final TableView tableView = TableView.builder( + verticalDetails: ScrollableDetails.vertical( + reverse: true, + controller: verticalController, + ), + horizontalDetails: ScrollableDetails.horizontal( + reverse: true, + controller: horizontalController, + ), + columnCount: 20, + rowCount: 20, + pinnedRowCount: 2, + pinnedColumnCount: 2, + columnBuilder: (_) => span, + rowBuilder: (_) => span, + cellBuilder: (_, TableVicinity vicinity) { + return TableViewCell( + columnMergeStart: + bothMerged[vicinity]?.start ?? columnMerged[vicinity]?.start, + columnMergeSpan: + bothMerged[vicinity]?.span ?? columnMerged[vicinity]?.span, + rowMergeStart: + bothMerged[vicinity]?.start ?? rowMerged[vicinity]?.start, + rowMergeSpan: + bothMerged[vicinity]?.span ?? rowMerged[vicinity]?.span, + child: Text( + 'R${bothMerged[vicinity]?.start ?? rowMerged[vicinity]?.start ?? vicinity.row}:' + 'C${bothMerged[vicinity]?.start ?? columnMerged[vicinity]?.start ?? vicinity.column}', + ), + ); + }, + ); + await tester.pumpWidget(MaterialApp(home: tableView)); + await tester.pumpAndSettle(); + + expect(verticalController.position.pixels, 0.0); + expect(horizontalController.position.pixels, 0.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(650.0, 450.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(500.0, 525.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(275.0, 450.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(725.0, 300.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(650.0, 75.0, 75.0, 225.0), + ); + + verticalController.jumpTo(10.0); + await tester.pumpAndSettle(); + expect(verticalController.position.pixels, 10.0); + expect(horizontalController.position.pixels, 0.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(650.0, 450.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(500.0, 525.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(275.0, 450.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(725.0, 310.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(650.0, 85.0, 75.0, 225.0), + ); + + horizontalController.jumpTo(10.0); + await tester.pumpAndSettle(); + expect(verticalController.position.pixels, 10.0); + expect(horizontalController.position.pixels, 10.0); + expect( + tester.getRect(find.text('R0:C0')), + const Rect.fromLTWH(650.0, 450.0, 150.0, 150.0), + ); + expect( + tester.getRect(find.text('R0:C2')), + const Rect.fromLTWH(510.0, 525.0, 150.0, 75.0), + ); + expect( + tester.getRect(find.text('R1:C4')), + const Rect.fromLTWH(285.0, 450.0, 225.0, 75.0), + ); + expect( + tester.getRect(find.text('R2:C0')), + const Rect.fromLTWH(725.0, 310.0, 75.0, 150.0), + ); + expect( + tester.getRect(find.text('R4:C1')), + const Rect.fromLTWH(650.0, 85.0, 75.0, 225.0), + ); + }); + }); }); }