diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index 6d84270e8b6..2f031898ab4 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.4.1 + +* Adds warnings for TableView pinned rows and columns that exceed the viewport dimensions. + ## 0.4.0 * Added `alignment` property to `TableView` and `TreeView` to align content within the viewport when it is smaller than the viewport extent. 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 66e5cf5ca5c..b09a7898e85 100644 --- a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart +++ b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart @@ -429,13 +429,6 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { ); } - // TODO(Piinks): Pinned rows/cols do not account for what is visible on the - // screen. Ostensibly, we would not want to have pinned rows/columns that - // extend beyond the viewport, we would never see them as they would never - // scroll into view. So this currently implementation is fairly assuming - // we will never have rows/cols that are outside of the viewport. We should - // maybe add an assertion for this during layout. - // https://github.com/flutter/flutter/issues/136833 int? get _lastPinnedRow => delegate.pinnedRowCount > 0 ? delegate.pinnedRowCount - 1 : null; int? get _lastPinnedColumn => @@ -448,6 +441,49 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { ? _columnMetrics[_lastPinnedColumn]!.trailingOffset : 0.0; + void _debugCheckPinnedExtent() { + assert(() { + if (_pinnedColumnsExtent > viewportDimension.width) { + debugPrint( + 'TableView has pinned columns with a total width of ' + '$_pinnedColumnsExtent, which exceeds the viewport width of ' + '${viewportDimension.width}. This will prevent unpinned columns ' + 'from being visible.', + ); + } else if (_pinnedColumnsExtent == viewportDimension.width) { + final bool hasUnpinnedColumns = + delegate.columnCount == null || + delegate.columnCount! > delegate.pinnedColumnCount; + if (hasUnpinnedColumns) { + debugPrint( + 'TableView has pinned columns that fully consume the viewport width. ' + 'Unpinned columns will not be visible.', + ); + } + } + + if (_pinnedRowsExtent > viewportDimension.height) { + debugPrint( + 'TableView has pinned rows with a total height of ' + '$_pinnedRowsExtent, which exceeds the viewport height of ' + '${viewportDimension.height}. This will prevent unpinned rows ' + 'from being visible.', + ); + } else if (_pinnedRowsExtent == viewportDimension.height) { + final bool hasUnpinnedRows = + delegate.rowCount == null || + delegate.rowCount! > delegate.pinnedRowCount; + if (hasUnpinnedRows) { + debugPrint( + 'TableView has pinned rows that fully consume the viewport height. ' + 'Unpinned rows will not be visible.', + ); + } + } + return true; + }()); + } + @override TableViewParentData parentDataOf(RenderBox child) => super.parentDataOf(child) as TableViewParentData; @@ -892,6 +928,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { _updateColumnMetrics(); _updateRowMetrics(); _updateScrollBounds(); + _debugCheckPinnedExtent(); } else { // Updates the visible cells based on cached table metrics. _updateFirstAndLastVisibleCell(); diff --git a/packages/two_dimensional_scrollables/pubspec.yaml b/packages/two_dimensional_scrollables/pubspec.yaml index 8525e06bff9..8e6d131bc8e 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.4.0 +version: 0.4.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/pinned_extent_warning_test.dart b/packages/two_dimensional_scrollables/test/table_view/pinned_extent_warning_test.dart new file mode 100644 index 00000000000..af0d6625b6e --- /dev/null +++ b/packages/two_dimensional_scrollables/test/table_view/pinned_extent_warning_test.dart @@ -0,0 +1,241 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:two_dimensional_scrollables/two_dimensional_scrollables.dart'; + +void main() { + group('TableView pinned extent warnings', () { + testWidgets('Warns when pinned columns exceed viewport width', ( + WidgetTester tester, + ) async { + // Regression test for https://github.com/flutter/flutter/issues/136833 + final log = []; + final DebugPrintCallback oldDebugPrint = debugPrint; + debugPrint = (String? message, {int? wrapWidth}) { + log.add(message!); + }; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + width: 200, + height: 400, + child: TableView.builder( + columnCount: 5, + rowCount: 5, + pinnedColumnCount: 3, + columnBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + rowBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + cellBuilder: (BuildContext context, TableVicinity vicinity) => + const TableViewCell(child: SizedBox.shrink()), + ), + ), + ), + ), + ); + + // Pinned columns extent = 300 (3 * 100), viewport width = 200. + // A warning is expected because the pinned columns are wider than the + // viewport, meaning even the pinned content cannot be fully displayed. + expect( + log, + contains( + matches( + r'TableView has pinned columns with a total width of 300(\.0)?, which exceeds the viewport width of 200(\.0)?', + ), + ), + ); + debugPrint = oldDebugPrint; + }); + + testWidgets('Warns when pinned rows exceed viewport height', ( + WidgetTester tester, + ) async { + // Regression test for https://github.com/flutter/flutter/issues/136833 + final log = []; + final DebugPrintCallback oldDebugPrint = debugPrint; + debugPrint = (String? message, {int? wrapWidth}) { + log.add(message!); + }; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + width: 400, + height: 200, + child: TableView.builder( + columnCount: 5, + rowCount: 5, + pinnedRowCount: 3, + columnBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + rowBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + cellBuilder: (BuildContext context, TableVicinity vicinity) => + const TableViewCell(child: SizedBox.shrink()), + ), + ), + ), + ), + ); + + // Pinned rows extent = 300 (3 * 100), viewport height = 200. + // A warning is expected because the pinned rows are taller than the + // viewport, meaning even the pinned content cannot be fully displayed. + expect( + log, + contains( + matches( + r'TableView has pinned rows with a total height of 300(\.0)?, which exceeds the viewport height of 200(\.0)?', + ), + ), + ); + debugPrint = oldDebugPrint; + }); + + testWidgets( + 'Warns when pinned columns fully consume viewport width and there are unpinned columns', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/136833 + final log = []; + final DebugPrintCallback oldDebugPrint = debugPrint; + debugPrint = (String? message, {int? wrapWidth}) { + log.add(message!); + }; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + width: 200, + height: 400, + child: TableView.builder( + columnCount: 3, + rowCount: 5, + pinnedColumnCount: 2, + columnBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + rowBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + cellBuilder: (BuildContext context, TableVicinity vicinity) => + const TableViewCell(child: SizedBox.shrink()), + ), + ), + ), + ), + ); + + // Pinned columns extent = 200 (2 * 100), viewport width = 200. + // There is 1 unpinned column (columnCount: 3, pinnedColumnCount: 2). + // Since the pinned columns take up the entire viewport width, the + // unpinned column will never be visible during scrolling. + expect( + log, + contains( + 'TableView has pinned columns that fully consume the viewport width. Unpinned columns will not be visible.', + ), + ); + debugPrint = oldDebugPrint; + }, + ); + + testWidgets( + 'Warns when pinned rows fully consume viewport height and there are unpinned rows', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/136833 + final log = []; + final DebugPrintCallback oldDebugPrint = debugPrint; + debugPrint = (String? message, {int? wrapWidth}) { + log.add(message!); + }; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + width: 400, + height: 200, + child: TableView.builder( + columnCount: 5, + rowCount: 3, + pinnedRowCount: 2, + columnBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + rowBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + cellBuilder: (BuildContext context, TableVicinity vicinity) => + const TableViewCell(child: SizedBox.shrink()), + ), + ), + ), + ), + ); + + // Pinned rows extent = 200 (2 * 100), viewport height = 200. + // There is 1 unpinned row (rowCount: 3, pinnedRowCount: 2). + // Since the pinned rows take up the entire viewport height, the + // unpinned row will never be visible during scrolling. + expect( + log, + contains( + 'TableView has pinned rows that fully consume the viewport height. Unpinned rows will not be visible.', + ), + ); + debugPrint = oldDebugPrint; + }, + ); + + testWidgets( + 'Does not warn when all columns are pinned even if they consume viewport', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/136833 + final log = []; + final DebugPrintCallback oldDebugPrint = debugPrint; + debugPrint = (String? message, {int? wrapWidth}) { + log.add(message!); + }; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + width: 200, + height: 400, + child: TableView.builder( + columnCount: 2, + rowCount: 5, + pinnedColumnCount: 2, + columnBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + rowBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(100)), + cellBuilder: (BuildContext context, TableVicinity vicinity) => + const TableViewCell(child: SizedBox.shrink()), + ), + ), + ), + ), + ); + + // Pinned columns extent = 200 (2 * 100), viewport width = 200. + // Although the pinned columns fully consume the viewport width, + // ALL columns are pinned (columnCount: 2, pinnedColumnCount: 2). + // Since there are no unpinned columns, no warning is issued about + // unpinned columns being hidden. + expect( + log, + isNot(contains(contains('Unpinned columns will not be visible'))), + ); + debugPrint = oldDebugPrint; + }, + ); + }); +}