Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/two_dimensional_scrollables/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.4.2

* Fixes an issue where merged cells would unmerge when the first cell was overlaid by a pinned row or column.

## 0.4.1

* Adds warnings for TableView pinned rows and columns that exceed the viewport dimensions.
Expand Down
26 changes: 19 additions & 7 deletions packages/two_dimensional_scrollables/lib/src/table_view/table.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,19 @@ import 'table_span.dart';
/// and columns through merging. The table supports lazy rendering and will only
/// instantiate those cells that are currently visible in the table's viewport
/// and those that extend into the [cacheExtent]. Therefore, when merging cells
/// in a [TableView], the same child must be returned from every vicinity the
/// merged cell contains. The `build` method will only be called once for a
/// merged cell, but since the table's children are lazily laid out, returning
/// the same child ensures the merged cell can be built no matter which part of
/// it is visible.
/// in a [TableView], the same child with the same merge information must be
/// returned from every vicinity the merged cell contains. The `build` method
/// will only be called once for a merged cell, but since the table's children
/// are lazily laid out, returning the same child and merge information ensures
/// the merged cell can be built no matter which part of it is visible.
///
/// For example, if a cell is configured to span 3 columns, starting at column 1,
/// the [cellBuilder] must return a [TableViewCell] with the same [child],
/// [columnMergeStart] as 1, and [columnMergeSpan] as 3 for all three
/// [TableVicinity]s (column 1, 2, and 3). If the merge information is only
/// provided for the first vicinity (column 1), and that vicinity is scrolled
/// out of the viewport and [cacheExtent], the table will not know the following
/// vicinities (column 2 and 3) are part of a merge and will "unmerge" them.
///
/// The layout of the table (e.g. how many rows/columns there are and their
/// extents) as well as the content of the individual cells is defined by
Expand Down Expand Up @@ -380,7 +388,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport {
// Where column layout begins, potentially outside of the visible area.
double get _targetLeadingColumnPixel {
return clampDouble(
horizontalOffset.pixels - cacheExtent,
horizontalOffset.pixels - math.max(_pinnedColumnsExtent, cacheExtent),
0,
double.infinity,
);
Expand All @@ -398,7 +406,11 @@ class RenderTableViewport extends RenderTwoDimensionalViewport {
bool get _rowsAreInfinite => delegate.rowCount == null;
// Where row layout begins, potentially outside of the visible area.
double get _targetLeadingRowPixel {
return clampDouble(verticalOffset.pixels - cacheExtent, 0, double.infinity);
return clampDouble(
verticalOffset.pixels - math.max(_pinnedRowsExtent, cacheExtent),
0,
double.infinity,
);
}

// How far rows should be laid out in a given frame.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,21 @@ class TableViewParentData extends TwoDimensionalViewportParentData {
/// Creates a cell of the [TableView], along with information regarding merged
/// cells and [RepaintBoundary]s.
///
/// When merging cells in a [TableView], the same child should be returned from
/// every vicinity the merged cell contains. The `build` method will only be
/// called once for a merged cell, but since the table's children are lazily
/// laid out, returning the same child ensures the merged cell can be built no
/// matter which part of it is visible.
/// When merging cells in a [TableView], the same child with the same merge
/// information must be returned from every vicinity the merged cell contains.
/// The `build` method will only be called once for a merged cell, but since
/// the table's children are lazily laid out, returning the same child and merge
/// information ensures the merged cell can be built no matter which part of it
/// is visible.
///
/// For example, if a cell is configured to span 3 columns, starting at column 1,
/// the `cellBuilder` of the [TableView] must return a [TableViewCell] with the
/// same [child], [columnMergeStart] as 1, and [columnMergeSpan] as 3 for all
/// three [TableVicinity]s (column 1, 2, and 3). If the merge information is
/// only provided for the first vicinity (column 1), and that vicinity is
/// scrolled out of the viewport and [cacheExtent], the table will not know the
/// following vicinities (column 2 and 3) are part of a merge and will "unmerge"
/// them.
class TableViewCell extends StatelessWidget {
/// Creates a widget that controls how a child of a [TableView] spans across
/// multiple rows or columns.
Expand Down
2 changes: 1 addition & 1 deletion packages/two_dimensional_scrollables/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: two_dimensional_scrollables
description: Widgets that scroll using the two dimensional scrolling foundation.
version: 0.4.1
version: 0.4.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+

Expand Down
128 changes: 128 additions & 0 deletions packages/two_dimensional_scrollables/test/table_view/table_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4176,6 +4176,134 @@ void main() {
},
);

testWidgets(
'Merged cells should not unmerge when the first cell is overlaid by a pinned column',
(WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/174862
final horizontalController = ScrollController();
addTearDown(horizontalController.dispose);

await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: SizedBox(
width: 400,
height: 400,
child: TableView.builder(
cacheExtent: 0.0,
horizontalDetails: ScrollableDetails.horizontal(
controller: horizontalController,
),
pinnedColumnCount: 1,
columnCount: 10,
rowCount: 10,
columnBuilder: (int index) => TableSpan(
extent: FixedTableSpanExtent(index == 0 ? 100 : 50),
),
rowBuilder: (int index) =>
const TableSpan(extent: FixedTableSpanExtent(50)),
cellBuilder: (BuildContext context, TableVicinity vicinity) {
// Merged cell spanning columns 1, 2, and 3.
// ONLY providing merge info for column 1, which is what the user reported.
Comment thread
Piinks marked this conversation as resolved.
Outdated
final isColumn1 = vicinity.column == 1;
return TableViewCell(
columnMergeStart: isColumn1 ? 1 : null,
columnMergeSpan: isColumn1 ? 3 : null,
child: Center(
child: Text('Cell ${vicinity.column},${vicinity.row}'),
),
);
},
),
),
),
),
);

// Initially, column 1 is visible next to pinned column 0.
expect(find.text('Cell 1,0'), findsOneWidget);
// Column 2 and 3 should be part of the merge, so they are not built.
expect(find.text('Cell 2,0'), findsNothing);
expect(find.text('Cell 3,0'), findsNothing);

// Scroll horizontally so that column 1 is entirely behind pinned column 0.
// Pinned column 0 is 0 to 100.
// Unpinned content starts at 100 (Column 1).
// Scroll 100 pixels.
// Content at 0 (start of column 1) moves to absolute 100 - 100 = 0.
// Content at 50 (end of column 1) moves to absolute 100 + (50 - 100) = 50.
// So column 1 (0 to 50) is entirely covered by pinned column 0.
// Column 2 (50 to 100) is also covered.
// Column 3 (100 to 150) is the first visible in unpinned area.
horizontalController.jumpTo(100);
await tester.pump();

// With the fix, column 1 is still built because it is under the pinned area.
// Since column 1 is built, its merge info is found and applied to columns 2 and 3.
expect(find.text('Cell 1,0'), findsOneWidget);
expect(find.text('Cell 2,0'), findsNothing);
expect(find.text('Cell 3,0'), findsNothing);
},
);

testWidgets(
'Merged cells should not unmerge when the first cell is overlaid by a pinned row',
(WidgetTester tester) async {
final verticalController = ScrollController();
addTearDown(verticalController.dispose);

await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: SizedBox(
width: 400,
height: 400,
child: TableView.builder(
cacheExtent: 0.0,
verticalDetails: ScrollableDetails.vertical(
controller: verticalController,
),
pinnedRowCount: 1,
columnCount: 10,
rowCount: 10,
columnBuilder: (int index) =>
const TableSpan(extent: FixedTableSpanExtent(50)),
rowBuilder: (int index) => TableSpan(
extent: FixedTableSpanExtent(index == 0 ? 100 : 50),
),
cellBuilder: (BuildContext context, TableVicinity vicinity) {
// Merged cell spanning rows 1, 2, and 3.
final isRow1 = vicinity.row == 1;
return TableViewCell(
rowMergeStart: isRow1 ? 1 : null,
rowMergeSpan: isRow1 ? 3 : null,
child: Center(
child: Text('Cell ${vicinity.column},${vicinity.row}'),
),
);
},
),
),
),
),
);

// Initially, row 1 is visible below pinned row 0.
expect(find.text('Cell 0,1'), findsOneWidget);
expect(find.text('Cell 0,2'), findsNothing);
expect(find.text('Cell 0,3'), findsNothing);

// Scroll vertically so that row 1 is entirely behind pinned row 0.
verticalController.jumpTo(100);
await tester.pump();

// Row 1 should still be built, maintaining the merge.
expect(find.text('Cell 0,1'), findsOneWidget);
expect(find.text('Cell 0,2'), findsNothing);
expect(find.text('Cell 0,3'), findsNothing);
},
);

testWidgets(
'Table does not crash when focusing outside of the table while focused text field is not in the view',
(WidgetTester tester) async {
Expand Down
Loading