From 3681b27a47ab713e6903b2208b46db5dcb49d60a Mon Sep 17 00:00:00 2001 From: David Neuy Date: Thu, 23 Feb 2023 20:32:29 +0100 Subject: [PATCH] Fix DataCell overflows when cell height is large by adding dataRowMinHeight, dataRowMaxHeight props. (#114338) * Fix DataCell overflows when cell height is large by adding dataRowMinHeight, dataRowMaxHeight props. * Fix DataCell overflows when cell height is large by adding dataRowMinHeight, dataRowMaxHeight props - add tests. * Fix analysis errors * Review changes. * Add asserts for dataRowMinHeight and dataRowMaxHeight * Add asserts for dataRowMinHeight and dataRowMaxHeight * Make dataRowHeight a computed getter * Remove get only dataRowHeight from hashCode... * Update deprecated after * Add new line at end of AUTHORS * Apply suggestions from code review * Update packages/flutter/test/material/data_table_test.dart --------- Co-authored-by: Kate Lovett --- AUTHORS | 1 + .../flutter/lib/src/material/data_table.dart | 50 +++++- .../lib/src/material/data_table_theme.dart | 46 ++++- .../src/material/paginated_data_table.dart | 34 +++- .../test/material/data_table_test.dart | 138 +++++++++++++- .../test/material/data_table_theme_test.dart | 169 +++++++++++++++--- .../material/paginated_data_table_test.dart | 13 +- 7 files changed, 405 insertions(+), 46 deletions(-) diff --git a/AUTHORS b/AUTHORS index 2ba77aacf9..737955348b 100644 --- a/AUTHORS +++ b/AUTHORS @@ -101,5 +101,6 @@ Junhua Lin <1075209054@qq.com> Tomasz Gucio Jason C.H Hubert Jóźwiak +David Neuy Eli Albert Jan Kuß diff --git a/packages/flutter/lib/src/material/data_table.dart b/packages/flutter/lib/src/material/data_table.dart index ceb5712f61..7330fa8043 100644 --- a/packages/flutter/lib/src/material/data_table.dart +++ b/packages/flutter/lib/src/material/data_table.dart @@ -392,7 +392,13 @@ class DataTable extends StatelessWidget { this.onSelectAll, this.decoration, this.dataRowColor, - this.dataRowHeight, + @Deprecated( + 'Migrate to use dataRowMinHeight and dataRowMaxHeight instead. ' + 'This feature was deprecated after v3.7.0-5.0.pre.', + ) + double? dataRowHeight, + double? dataRowMinHeight, + double? dataRowMaxHeight, this.dataTextStyle, this.headingRowColor, this.headingRowHeight, @@ -410,6 +416,11 @@ class DataTable extends StatelessWidget { assert(sortColumnIndex == null || (sortColumnIndex >= 0 && sortColumnIndex < columns.length)), assert(!rows.any((DataRow row) => row.cells.length != columns.length)), assert(dividerThickness == null || dividerThickness >= 0), + assert(dataRowMinHeight == null || dataRowMaxHeight == null || dataRowMaxHeight >= dataRowMinHeight), + assert(dataRowHeight == null || (dataRowMinHeight == null && dataRowMaxHeight == null), + 'dataRowHeight ($dataRowHeight) must not be set if dataRowMinHeight ($dataRowMinHeight) or dataRowMaxHeight ($dataRowMaxHeight) are set.'), + dataRowMinHeight = dataRowHeight ?? dataRowMinHeight, + dataRowMaxHeight = dataRowHeight ?? dataRowMaxHeight, _onlyTextColumn = _initOnlyTextColumn(columns); /// The configuration and labels for the columns in the table. @@ -504,7 +515,29 @@ class DataTable extends StatelessWidget { /// If null, [DataTableThemeData.dataRowHeight] is used. This value defaults /// to [kMinInteractiveDimension] to adhere to the Material Design /// specifications. - final double? dataRowHeight; + @Deprecated( + 'Migrate to use dataRowMinHeight and dataRowMaxHeight instead. ' + 'This feature was deprecated after v3.7.0-5.0.pre.', + ) + double? get dataRowHeight => dataRowMinHeight == dataRowMaxHeight ? dataRowMinHeight : null; + + /// {@template flutter.material.dataTable.dataRowMinHeight} + /// The minimum height of each row (excluding the row that contains column headings). + /// {@endtemplate} + /// + /// If null, [DataTableThemeData.dataRowMinHeight] is used. This value defaults + /// to [kMinInteractiveDimension] to adhere to the Material Design + /// specifications. + final double? dataRowMinHeight; + + /// {@template flutter.material.dataTable.dataRowMaxHeight} + /// The maximum height of each row (excluding the row that contains column headings). + /// {@endtemplate} + /// + /// If null, [DataTableThemeData.dataRowMaxHeight] is used. This value defaults + /// to [kMinInteractiveDimension] to adhere to the Material Design + /// specifications. + final double? dataRowMaxHeight; /// {@template flutter.material.dataTable.dataTextStyle} /// The text style for data rows. @@ -841,13 +874,17 @@ class DataTable extends StatelessWidget { ?? dataTableTheme.dataTextStyle ?? themeData.dataTableTheme.dataTextStyle ?? themeData.textTheme.bodyMedium!; - final double effectiveDataRowHeight = dataRowHeight - ?? dataTableTheme.dataRowHeight - ?? themeData.dataTableTheme.dataRowHeight + final double effectiveDataRowMinHeight = dataRowMinHeight + ?? dataTableTheme.dataRowMinHeight + ?? themeData.dataTableTheme.dataRowMinHeight + ?? kMinInteractiveDimension; + final double effectiveDataRowMaxHeight = dataRowMaxHeight + ?? dataTableTheme.dataRowMaxHeight + ?? themeData.dataTableTheme.dataRowMaxHeight ?? kMinInteractiveDimension; label = Container( padding: padding, - height: effectiveDataRowHeight, + constraints: BoxConstraints(minHeight: effectiveDataRowMinHeight, maxHeight: effectiveDataRowMaxHeight), alignment: numeric ? Alignment.centerRight : AlignmentDirectional.centerStart, child: DefaultTextStyle( style: effectiveDataTextStyle.copyWith( @@ -1063,6 +1100,7 @@ class DataTable extends StatelessWidget { clipBehavior: clipBehavior, child: Table( columnWidths: tableColumns.asMap(), + defaultVerticalAlignment: TableCellVerticalAlignment.middle, children: tableRows, border: border, ), diff --git a/packages/flutter/lib/src/material/data_table_theme.dart b/packages/flutter/lib/src/material/data_table_theme.dart index 578edd2399..3d3a155f52 100644 --- a/packages/flutter/lib/src/material/data_table_theme.dart +++ b/packages/flutter/lib/src/material/data_table_theme.dart @@ -40,7 +40,13 @@ class DataTableThemeData with Diagnosticable { const DataTableThemeData({ this.decoration, this.dataRowColor, - this.dataRowHeight, + @Deprecated( + 'Migrate to use dataRowMinHeight and dataRowMaxHeight instead. ' + 'This feature was deprecated after v3.7.0-5.0.pre.', + ) + double? dataRowHeight, + double? dataRowMinHeight, + double? dataRowMaxHeight, this.dataTextStyle, this.headingRowColor, this.headingRowHeight, @@ -49,7 +55,11 @@ class DataTableThemeData with Diagnosticable { this.columnSpacing, this.dividerThickness, this.checkboxHorizontalMargin, - }); + }) : assert(dataRowMinHeight == null || dataRowMaxHeight == null || dataRowMaxHeight >= dataRowMinHeight), + assert(dataRowHeight == null || (dataRowMinHeight == null && dataRowMaxHeight == null), + 'dataRowHeight ($dataRowHeight) must not be set if dataRowMinHeight ($dataRowMinHeight) or dataRowMaxHeight ($dataRowMaxHeight) are set.'), + dataRowMinHeight = dataRowHeight ?? dataRowMinHeight, + dataRowMaxHeight = dataRowHeight ?? dataRowMaxHeight; /// {@macro flutter.material.dataTable.decoration} final Decoration? decoration; @@ -59,7 +69,17 @@ class DataTableThemeData with Diagnosticable { final MaterialStateProperty? dataRowColor; /// {@macro flutter.material.dataTable.dataRowHeight} - final double? dataRowHeight; + @Deprecated( + 'Migrate to use dataRowMinHeight and dataRowMaxHeight instead. ' + 'This feature was deprecated after v3.7.0-5.0.pre.', + ) + double? get dataRowHeight => dataRowMinHeight == dataRowMaxHeight ? dataRowMinHeight : null; + + /// {@macro flutter.material.dataTable.dataRowMinHeight} + final double? dataRowMinHeight; + + /// {@macro flutter.material.dataTable.dataRowMaxHeight} + final double? dataRowMaxHeight; /// {@macro flutter.material.dataTable.dataTextStyle} final TextStyle? dataTextStyle; @@ -91,7 +111,13 @@ class DataTableThemeData with Diagnosticable { DataTableThemeData copyWith({ Decoration? decoration, MaterialStateProperty? dataRowColor, + @Deprecated( + 'Migrate to use dataRowMinHeight and dataRowMaxHeight instead. ' + 'This feature was deprecated after v3.7.0-5.0.pre.', + ) double? dataRowHeight, + double? dataRowMinHeight, + double? dataRowMaxHeight, TextStyle? dataTextStyle, MaterialStateProperty? headingRowColor, double? headingRowHeight, @@ -105,6 +131,8 @@ class DataTableThemeData with Diagnosticable { decoration: decoration ?? this.decoration, dataRowColor: dataRowColor ?? this.dataRowColor, dataRowHeight: dataRowHeight ?? this.dataRowHeight, + dataRowMinHeight: dataRowMinHeight ?? this.dataRowMinHeight, + dataRowMaxHeight: dataRowMaxHeight ?? this.dataRowMaxHeight, dataTextStyle: dataTextStyle ?? this.dataTextStyle, headingRowColor: headingRowColor ?? this.headingRowColor, headingRowHeight: headingRowHeight ?? this.headingRowHeight, @@ -128,7 +156,8 @@ class DataTableThemeData with Diagnosticable { return DataTableThemeData( decoration: Decoration.lerp(a.decoration, b.decoration, t), dataRowColor: MaterialStateProperty.lerp(a.dataRowColor, b.dataRowColor, t, Color.lerp), - dataRowHeight: lerpDouble(a.dataRowHeight, b.dataRowHeight, t), + dataRowMinHeight: lerpDouble(a.dataRowMinHeight, b.dataRowMinHeight, t), + dataRowMaxHeight: lerpDouble(a.dataRowMaxHeight, b.dataRowMaxHeight, t), dataTextStyle: TextStyle.lerp(a.dataTextStyle, b.dataTextStyle, t), headingRowColor: MaterialStateProperty.lerp(a.headingRowColor, b.headingRowColor, t, Color.lerp), headingRowHeight: lerpDouble(a.headingRowHeight, b.headingRowHeight, t), @@ -144,7 +173,8 @@ class DataTableThemeData with Diagnosticable { int get hashCode => Object.hash( decoration, dataRowColor, - dataRowHeight, + dataRowMinHeight, + dataRowMaxHeight, dataTextStyle, headingRowColor, headingRowHeight, @@ -166,7 +196,8 @@ class DataTableThemeData with Diagnosticable { return other is DataTableThemeData && other.decoration == decoration && other.dataRowColor == dataRowColor - && other.dataRowHeight == dataRowHeight + && other.dataRowMinHeight == dataRowMinHeight + && other.dataRowMaxHeight == dataRowMaxHeight && other.dataTextStyle == dataTextStyle && other.headingRowColor == headingRowColor && other.headingRowHeight == headingRowHeight @@ -182,7 +213,8 @@ class DataTableThemeData with Diagnosticable { super.debugFillProperties(properties); properties.add(DiagnosticsProperty('decoration', decoration, defaultValue: null)); properties.add(DiagnosticsProperty>('dataRowColor', dataRowColor, defaultValue: null)); - properties.add(DoubleProperty('dataRowHeight', dataRowHeight, defaultValue: null)); + properties.add(DoubleProperty('dataRowMinHeight', dataRowMinHeight, defaultValue: null)); + properties.add(DoubleProperty('dataRowMaxHeight', dataRowMaxHeight, defaultValue: null)); properties.add(DiagnosticsProperty('dataTextStyle', dataTextStyle, defaultValue: null)); properties.add(DiagnosticsProperty>('headingRowColor', headingRowColor, defaultValue: null)); properties.add(DoubleProperty('headingRowHeight', headingRowHeight, defaultValue: null)); diff --git a/packages/flutter/lib/src/material/paginated_data_table.dart b/packages/flutter/lib/src/material/paginated_data_table.dart index 74c0fa6459..56dc56fbb0 100644 --- a/packages/flutter/lib/src/material/paginated_data_table.dart +++ b/packages/flutter/lib/src/material/paginated_data_table.dart @@ -71,7 +71,13 @@ class PaginatedDataTable extends StatefulWidget { this.sortColumnIndex, this.sortAscending = true, this.onSelectAll, - this.dataRowHeight = kMinInteractiveDimension, + @Deprecated( + 'Migrate to use dataRowMinHeight and dataRowMaxHeight instead. ' + 'This feature was deprecated after v3.7.0-5.0.pre.', + ) + double? dataRowHeight, + double? dataRowMinHeight, + double? dataRowMaxHeight, this.headingRowHeight = 56.0, this.horizontalMargin = 24.0, this.columnSpacing = 56.0, @@ -91,6 +97,11 @@ class PaginatedDataTable extends StatefulWidget { }) : assert(actions == null || (header != null)), assert(columns.isNotEmpty), assert(sortColumnIndex == null || (sortColumnIndex >= 0 && sortColumnIndex < columns.length)), + assert(dataRowMinHeight == null || dataRowMaxHeight == null || dataRowMaxHeight >= dataRowMinHeight), + assert(dataRowHeight == null || (dataRowMinHeight == null && dataRowMaxHeight == null), + 'dataRowHeight ($dataRowHeight) must not be set if dataRowMinHeight ($dataRowMinHeight) or dataRowMaxHeight ($dataRowMaxHeight) are set.'), + dataRowMinHeight = dataRowHeight ?? dataRowMinHeight ?? kMinInteractiveDimension, + dataRowMaxHeight = dataRowHeight ?? dataRowMaxHeight ?? kMinInteractiveDimension, assert(rowsPerPage > 0), assert(() { if (onRowsPerPageChanged != null) { @@ -147,7 +158,23 @@ class PaginatedDataTable extends StatefulWidget { /// /// This value is optional and defaults to kMinInteractiveDimension if not /// specified. - final double dataRowHeight; + @Deprecated( + 'Migrate to use dataRowMinHeight and dataRowMaxHeight instead. ' + 'This feature was deprecated after v3.7.0-5.0.pre.', + ) + double? get dataRowHeight => dataRowMinHeight == dataRowMaxHeight ? dataRowMinHeight : null; + + /// The minimum height of each row (excluding the row that contains column headings). + /// + /// This value is optional and defaults to [kMinInteractiveDimension] if not + /// specified. + final double dataRowMinHeight; + + /// The maximum height of each row (excluding the row that contains column headings). + /// + /// This value is optional and defaults to kMinInteractiveDimension if not + /// specified. + final double dataRowMaxHeight; /// The height of the heading row. /// @@ -518,7 +545,8 @@ class PaginatedDataTableState extends State { // Make sure no decoration is set on the DataTable // from the theme, as its already wrapped in a Card. decoration: const BoxDecoration(), - dataRowHeight: widget.dataRowHeight, + dataRowMinHeight: widget.dataRowMinHeight, + dataRowMaxHeight: widget.dataRowMaxHeight, headingRowHeight: widget.headingRowHeight, horizontalMargin: widget.horizontalMargin, checkboxHorizontalMargin: widget.checkboxHorizontalMargin, diff --git a/packages/flutter/test/material/data_table_test.dart b/packages/flutter/test/material/data_table_test.dart index 177923290b..e8e2d61256 100644 --- a/packages/flutter/test/material/data_table_test.dart +++ b/packages/flutter/test/material/data_table_test.dart @@ -629,14 +629,16 @@ void main() { Widget buildCustomTable({ int? sortColumnIndex, bool sortAscending = true, - double dataRowHeight = 48.0, + double? dataRowMinHeight, + double? dataRowMaxHeight, double headingRowHeight = 56.0, }) { return DataTable( sortColumnIndex: sortColumnIndex, sortAscending: sortAscending, onSelectAll: (bool? value) {}, - dataRowHeight: dataRowHeight, + dataRowMinHeight: dataRowMinHeight, + dataRowMaxHeight: dataRowMaxHeight, headingRowHeight: headingRowHeight, columns: [ const DataColumn( @@ -712,7 +714,7 @@ void main() { Finder findFirstContainerFor(String text) => find.widgetWithText(Container, text).first; expect(tester.getSize(findFirstContainerFor('Name')).height, 56.0); - expect(tester.getSize(findFirstContainerFor('Frozen yogurt')).height, 48.0); + expect(tester.getSize(findFirstContainerFor('Frozen yogurt')).height, kMinInteractiveDimension); // CUSTOM VALUES await tester.pumpWidget(MaterialApp( @@ -726,14 +728,105 @@ void main() { expect(tester.getSize(findFirstContainerFor('Name')).height, 64.0); await tester.pumpWidget(MaterialApp( - home: Material(child: buildCustomTable(dataRowHeight: 30.0)), + home: Material(child: buildCustomTable(dataRowMinHeight: 30.0, dataRowMaxHeight: 30.0)), )); expect(tester.getSize(findFirstContainerFor('Frozen yogurt')).height, 30.0); await tester.pumpWidget(MaterialApp( - home: Material(child: buildCustomTable(dataRowHeight: 56.0)), + home: Material(child: buildCustomTable(dataRowMinHeight: 0.0, dataRowMaxHeight: double.infinity)), )); - expect(tester.getSize(findFirstContainerFor('Frozen yogurt')).height, 56.0); + expect(tester.getSize(findFirstContainerFor('Frozen yogurt')).height, greaterThan(0.0)); + }); + + testWidgets('DataTable custom row height one row taller than others', (WidgetTester tester) async { + const String multilineText = 'Line one.\nLine two.\nLine three.\nLine four.'; + + Widget buildCustomTable({ + double? dataRowMinHeight, + double? dataRowMaxHeight, + }) { + return DataTable( + dataRowMinHeight: dataRowMinHeight, + dataRowMaxHeight: dataRowMaxHeight, + columns: const [ + DataColumn( + label: Text('SingleRowColumn'), + ), + DataColumn( + label: Text('MultiRowColumn'), + ), + ], + rows: const [ + DataRow(cells: [ + DataCell(Text('Data')), + DataCell(Column(children: [ + Text(multilineText), + ])), + ]), + ], + ); + } + + Finder findFirstContainerFor(String text) => find.widgetWithText(Container, text).first; + + await tester.pumpWidget(MaterialApp( + home: Material(child: buildCustomTable(dataRowMinHeight: 0.0, dataRowMaxHeight: double.infinity)), + )); + + final double singleLineRowHeight = tester.getSize(findFirstContainerFor('Data')).height; + final double multilineRowHeight = tester.getSize(findFirstContainerFor(multilineText)).height; + + expect(multilineRowHeight, greaterThan(singleLineRowHeight)); + }); + + testWidgets('DataTable custom row height - separate test for deprecated dataRowHeight', (WidgetTester tester) async { + Widget buildCustomTable({ + double dataRowHeight = 48.0, + }) { + return DataTable( + onSelectAll: (bool? value) {}, + dataRowHeight: dataRowHeight, + columns: [ + const DataColumn( + label: Text('Name'), + tooltip: 'Name', + ), + DataColumn( + label: const Text('Calories'), + tooltip: 'Calories', + numeric: true, + onSort: (int columnIndex, bool ascending) {}, + ), + ], + rows: kDesserts.map((Dessert dessert) { + return DataRow( + key: ValueKey(dessert.name), + onSelectChanged: (bool? selected) {}, + cells: [ + DataCell( + Text(dessert.name), + ), + DataCell( + Text('${dessert.calories}'), + showEditIcon: true, + onTap: () {}, + ), + ], + ); + }).toList(), + ); + } + + // The finder matches with the Container of the cell content, as well as the + // Container wrapping the whole table. The first one is used to test row + // heights. + Finder findFirstContainerFor(String text) => find.widgetWithText(Container, text).first; + + // CUSTOM VALUES + await tester.pumpWidget(MaterialApp( + home: Material(child: buildCustomTable(dataRowHeight: 30.0)), + )); + expect(tester.getSize(findFirstContainerFor('Frozen yogurt')).height, 30.0); }); testWidgets('DataTable custom horizontal padding - checkbox', (WidgetTester tester) async { @@ -1946,4 +2039,37 @@ void main() { expect(material.clipBehavior, Clip.hardEdge); expect(material.borderRadius, borderRadius); }); + + testWidgets('DataTable dataRowMinHeight smaller or equal dataRowMaxHeight validation', (WidgetTester tester) async { + DataTable createDataTable() => + DataTable( + columns: const [DataColumn(label: Text('Column1'))], + rows: const [], + dataRowMinHeight: 2.0, + dataRowMaxHeight: 1.0, + ); + + expect(() => createDataTable(), throwsA(predicate((AssertionError e) => + e.toString().contains('dataRowMaxHeight >= dataRowMinHeight')))); + }); + + testWidgets('DataTable dataRowHeight is not used together with dataRowMinHeight or dataRowMaxHeight', (WidgetTester tester) async { + DataTable createDataTable({double? dataRowHeight, double? dataRowMinHeight, double? dataRowMaxHeight}) => + DataTable( + columns: const [DataColumn(label: Text('Column1'))], + rows: const [], + dataRowHeight: dataRowHeight, + dataRowMinHeight: dataRowMinHeight, + dataRowMaxHeight: dataRowMaxHeight, + ); + + expect(() => createDataTable(dataRowHeight: 1.0, dataRowMinHeight: 2.0, dataRowMaxHeight: 2.0), throwsA(predicate((AssertionError e) => + e.toString().contains('dataRowHeight == null || (dataRowMinHeight == null && dataRowMaxHeight == null)')))); + + expect(() => createDataTable(dataRowHeight: 1.0, dataRowMaxHeight: 2.0), throwsA(predicate((AssertionError e) => + e.toString().contains('dataRowHeight == null || (dataRowMinHeight == null && dataRowMaxHeight == null)')))); + + expect(() => createDataTable(dataRowHeight: 1.0, dataRowMinHeight: 2.0), throwsA(predicate((AssertionError e) => + e.toString().contains('dataRowHeight == null || (dataRowMinHeight == null && dataRowMaxHeight == null)')))); + }); } diff --git a/packages/flutter/test/material/data_table_theme_test.dart b/packages/flutter/test/material/data_table_theme_test.dart index 9880ba1a91..566533db6e 100644 --- a/packages/flutter/test/material/data_table_theme_test.dart +++ b/packages/flutter/test/material/data_table_theme_test.dart @@ -22,6 +22,8 @@ void main() { expect(themeData.decoration, null); expect(themeData.dataRowColor, null); expect(themeData.dataRowHeight, null); + expect(themeData.dataRowMinHeight, null); + expect(themeData.dataRowMaxHeight, null); expect(themeData.dataTextStyle, null); expect(themeData.headingRowColor, null); expect(themeData.headingRowHeight, null); @@ -35,6 +37,8 @@ void main() { expect(theme.data.decoration, null); expect(theme.data.dataRowColor, null); expect(theme.data.dataRowHeight, null); + expect(theme.data.dataRowMinHeight, null); + expect(theme.data.dataRowMaxHeight, null); expect(theme.data.dataTextStyle, null); expect(theme.data.headingRowColor, null); expect(theme.data.headingRowHeight, null); @@ -64,7 +68,8 @@ void main() { dataRowColor: MaterialStateProperty.resolveWith( (Set states) => const Color(0xfffffff1), ), - dataRowHeight: 51.0, + dataRowMinHeight: 41.0, + dataRowMaxHeight: 42.0, dataTextStyle: const TextStyle(fontSize: 12.0), headingRowColor: MaterialStateProperty.resolveWith( (Set states) => const Color(0xfffffff2), @@ -84,21 +89,22 @@ void main() { expect(description[0], 'decoration: BoxDecoration(color: Color(0xfffffff0))'); expect(description[1], "dataRowColor: Instance of '_MaterialStatePropertyWith'"); - expect(description[2], 'dataRowHeight: 51.0'); - expect(description[3], 'dataTextStyle: TextStyle(inherit: true, size: 12.0)'); - expect(description[4], "headingRowColor: Instance of '_MaterialStatePropertyWith'"); - expect(description[5], 'headingRowHeight: 52.0'); - expect(description[6], 'headingTextStyle: TextStyle(inherit: true, size: 14.0)'); - expect(description[7], 'horizontalMargin: 3.0'); - expect(description[8], 'columnSpacing: 4.0'); - expect(description[9], 'dividerThickness: 5.0'); - expect(description[10], 'checkboxHorizontalMargin: 6.0'); + expect(description[2], 'dataRowMinHeight: 41.0'); + expect(description[3], 'dataRowMaxHeight: 42.0'); + expect(description[4], 'dataTextStyle: TextStyle(inherit: true, size: 12.0)'); + expect(description[5], "headingRowColor: Instance of '_MaterialStatePropertyWith'"); + expect(description[6], 'headingRowHeight: 52.0'); + expect(description[7], 'headingTextStyle: TextStyle(inherit: true, size: 14.0)'); + expect(description[8], 'horizontalMargin: 3.0'); + expect(description[9], 'columnSpacing: 4.0'); + expect(description[10], 'dividerThickness: 5.0'); + expect(description[11], 'checkboxHorizontalMargin: 6.0'); }); testWidgets('DataTable is themeable', (WidgetTester tester) async { const BoxDecoration decoration = BoxDecoration(color: Color(0xfffffff0)); const MaterialStateProperty dataRowColor = MaterialStatePropertyAll(Color(0xfffffff1)); - const double dataRowHeight = 51.0; + const double minMaxDataRowHeight = 41.0; const TextStyle dataTextStyle = TextStyle(fontSize: 12.5); const MaterialStateProperty headingRowColor = MaterialStatePropertyAll(Color(0xfffffff2)); const double headingRowHeight = 52.0; @@ -113,7 +119,8 @@ void main() { dataTableTheme: const DataTableThemeData( decoration: decoration, dataRowColor: dataRowColor, - dataRowHeight: dataRowHeight, + dataRowMinHeight: minMaxDataRowHeight, + dataRowMaxHeight: minMaxDataRowHeight, dataTextStyle: dataTextStyle, headingRowColor: headingRowColor, headingRowHeight: headingRowHeight, @@ -151,7 +158,7 @@ void main() { expect(dataRowTextStyle.fontSize, dataTextStyle.fontSize); expect(_tableRowBoxDecoration(tester: tester, index: 1).color, dataRowColor.resolve({})); expect(_tableRowBoxDecoration(tester: tester, index: 1).border!.top.width, dividerThickness); - expect(tester.getSize(_findFirstContainerFor('Data')).height, dataRowHeight); + expect(tester.getSize(_findFirstContainerFor('Data')).height, minMaxDataRowHeight); final TextStyle headingRowTextStyle = tester.renderObject(find.text('A')).text.style!; expect(headingRowTextStyle.fontSize, headingTextStyle.fontSize); @@ -162,10 +169,44 @@ void main() { expect(tester.getTopLeft(find.text('Data 2')).dx - tester.getTopRight(find.text('Data')).dx, columnSpacing); }); + testWidgets('DataTable is themeable - separate test for deprecated dataRowHeight', (WidgetTester tester) async { + const double dataRowHeight = 51.0; + + await tester.pumpWidget( + MaterialApp( + theme: ThemeData( + dataTableTheme: const DataTableThemeData( + dataRowHeight: dataRowHeight, + ), + ), + home: Scaffold( + body: DataTable( + sortColumnIndex: 0, + columns: [ + DataColumn( + label: const Text('A'), + onSort: (int columnIndex, bool ascending) {}, + ), + const DataColumn(label: Text('B')), + ], + rows: const [ + DataRow(cells: [ + DataCell(Text('Data')), + DataCell(Text('Data 2')), + ]), + ], + ), + ), + ), + ); + + expect(tester.getSize(_findFirstContainerFor('Data')).height, dataRowHeight); + }); + testWidgets('DataTable properties are taken over the theme values', (WidgetTester tester) async { const BoxDecoration themeDecoration = BoxDecoration(color: Color(0xfffffff1)); const MaterialStateProperty themeDataRowColor = MaterialStatePropertyAll(Color(0xfffffff0)); - const double themeDataRowHeight = 50.0; + const double minMaxThemeDataRowHeight = 50.0; const TextStyle themeDataTextStyle = TextStyle(fontSize: 11.5); const MaterialStateProperty themeHeadingRowColor = MaterialStatePropertyAll(Color(0xfffffff1)); const double themeHeadingRowHeight = 51.0; @@ -176,7 +217,7 @@ void main() { const BoxDecoration decoration = BoxDecoration(color: Color(0xfffffff0)); const MaterialStateProperty dataRowColor = MaterialStatePropertyAll(Color(0xfffffff1)); - const double dataRowHeight = 51.0; + const double minMaxDataRowHeight = 51.0; const TextStyle dataTextStyle = TextStyle(fontSize: 12.5); const MaterialStateProperty headingRowColor = MaterialStatePropertyAll(Color(0xfffffff2)); const double headingRowHeight = 52.0; @@ -184,13 +225,15 @@ void main() { const double horizontalMargin = 3.0; const double columnSpacing = 4.0; const double dividerThickness = 5.0; + await tester.pumpWidget( MaterialApp( theme: ThemeData( dataTableTheme: const DataTableThemeData( decoration: themeDecoration, dataRowColor: themeDataRowColor, - dataRowHeight: themeDataRowHeight, + dataRowMinHeight: minMaxThemeDataRowHeight, + dataRowMaxHeight: minMaxThemeDataRowHeight, dataTextStyle: themeDataTextStyle, headingRowColor: themeHeadingRowColor, headingRowHeight: themeHeadingRowHeight, @@ -204,7 +247,8 @@ void main() { body: DataTable( decoration: decoration, dataRowColor: dataRowColor, - dataRowHeight: dataRowHeight, + dataRowMinHeight: minMaxDataRowHeight, + dataRowMaxHeight: minMaxDataRowHeight, dataTextStyle: dataTextStyle, headingRowColor: headingRowColor, headingRowHeight: headingRowHeight, @@ -238,7 +282,7 @@ void main() { expect(dataRowTextStyle.fontSize, dataTextStyle.fontSize); expect(_tableRowBoxDecoration(tester: tester, index: 1).color, dataRowColor.resolve({})); expect(_tableRowBoxDecoration(tester: tester, index: 1).border!.top.width, dividerThickness); - expect(tester.getSize(_findFirstContainerFor('Data')).height, dataRowHeight); + expect(tester.getSize(_findFirstContainerFor('Data')).height, minMaxDataRowHeight); final TextStyle headingRowTextStyle = tester.renderObject(find.text('A')).text.style!; expect(headingRowTextStyle.fontSize, headingTextStyle.fontSize); @@ -249,10 +293,46 @@ void main() { expect(tester.getTopLeft(find.text('Data 2')).dx - tester.getTopRight(find.text('Data')).dx, columnSpacing); }); + testWidgets('DataTable properties are taken over the theme values - separate test for deprecated dataRowHeight', (WidgetTester tester) async { + const double themeDataRowHeight = 50.0; + const double dataRowHeight = 51.0; + + await tester.pumpWidget( + MaterialApp( + theme: ThemeData( + dataTableTheme: const DataTableThemeData( + dataRowHeight: themeDataRowHeight, + ), + ), + home: Scaffold( + body: DataTable( + dataRowHeight: dataRowHeight, + sortColumnIndex: 0, + columns: [ + DataColumn( + label: const Text('A'), + onSort: (int columnIndex, bool ascending) {}, + ), + const DataColumn(label: Text('B')), + ], + rows: const [ + DataRow(cells: [ + DataCell(Text('Data')), + DataCell(Text('Data 2')), + ]), + ], + ), + ), + ), + ); + + expect(tester.getSize(_findFirstContainerFor('Data')).height, dataRowHeight); + }); + testWidgets('Local DataTableTheme can override global DataTableTheme', (WidgetTester tester) async { const BoxDecoration globalThemeDecoration = BoxDecoration(color: Color(0xfffffff1)); const MaterialStateProperty globalThemeDataRowColor = MaterialStatePropertyAll(Color(0xfffffff0)); - const double globalThemeDataRowHeight = 50.0; + const double minMaxGlobalThemeDataRowHeight = 50.0; const TextStyle globalThemeDataTextStyle = TextStyle(fontSize: 11.5); const MaterialStateProperty globalThemeHeadingRowColor = MaterialStatePropertyAll(Color(0xfffffff1)); const double globalThemeHeadingRowHeight = 51.0; @@ -263,7 +343,7 @@ void main() { const BoxDecoration localThemeDecoration = BoxDecoration(color: Color(0xfffffff0)); const MaterialStateProperty localThemeDataRowColor = MaterialStatePropertyAll(Color(0xfffffff1)); - const double localThemeDataRowHeight = 51.0; + const double minMaxLocalThemeDataRowHeight = 51.0; const TextStyle localThemeDataTextStyle = TextStyle(fontSize: 12.5); const MaterialStateProperty localThemeHeadingRowColor = MaterialStatePropertyAll(Color(0xfffffff2)); const double localThemeHeadingRowHeight = 52.0; @@ -278,7 +358,8 @@ void main() { dataTableTheme: const DataTableThemeData( decoration: globalThemeDecoration, dataRowColor: globalThemeDataRowColor, - dataRowHeight: globalThemeDataRowHeight, + dataRowMinHeight: minMaxGlobalThemeDataRowHeight, + dataRowMaxHeight: minMaxGlobalThemeDataRowHeight, dataTextStyle: globalThemeDataTextStyle, headingRowColor: globalThemeHeadingRowColor, headingRowHeight: globalThemeHeadingRowHeight, @@ -293,7 +374,8 @@ void main() { data: const DataTableThemeData( decoration: localThemeDecoration, dataRowColor: localThemeDataRowColor, - dataRowHeight: localThemeDataRowHeight, + dataRowMinHeight: minMaxLocalThemeDataRowHeight, + dataRowMaxHeight: minMaxLocalThemeDataRowHeight, dataTextStyle: localThemeDataTextStyle, headingRowColor: localThemeHeadingRowColor, headingRowHeight: localThemeHeadingRowHeight, @@ -330,7 +412,7 @@ void main() { expect(dataRowTextStyle.fontSize, localThemeDataTextStyle.fontSize); expect(_tableRowBoxDecoration(tester: tester, index: 1).color, localThemeDataRowColor.resolve({})); expect(_tableRowBoxDecoration(tester: tester, index: 1).border!.top.width, localThemeDividerThickness); - expect(tester.getSize(_findFirstContainerFor('Data')).height, localThemeDataRowHeight); + expect(tester.getSize(_findFirstContainerFor('Data')).height, minMaxLocalThemeDataRowHeight); final TextStyle headingRowTextStyle = tester.renderObject(find.text('A')).text.style!; expect(headingRowTextStyle.fontSize, localThemeHeadingTextStyle.fontSize); @@ -340,8 +422,49 @@ void main() { expect(tester.getTopLeft(find.text('A')).dx, localThemeHorizontalMargin); expect(tester.getTopLeft(find.text('Data 2')).dx - tester.getTopRight(find.text('Data')).dx, localThemeColumnSpacing); }); + + testWidgets('Local DataTableTheme can override global DataTableTheme - separate test for deprecated dataRowHeight', (WidgetTester tester) async { + const double globalThemeDataRowHeight = 50.0; + const double localThemeDataRowHeight = 51.0; + + await tester.pumpWidget( + MaterialApp( + theme: ThemeData( + dataTableTheme: const DataTableThemeData( + dataRowHeight: globalThemeDataRowHeight, + ), + ), + home: Scaffold( + body: DataTableTheme( + data: const DataTableThemeData( + dataRowHeight: localThemeDataRowHeight, + ), + child: DataTable( + sortColumnIndex: 0, + columns: [ + DataColumn( + label: const Text('A'), + onSort: (int columnIndex, bool ascending) {}, + ), + const DataColumn(label: Text('B')), + ], + rows: const [ + DataRow(cells: [ + DataCell(Text('Data')), + DataCell(Text('Data 2')), + ]), + ], + ), + ), + ), + ), + ); + + expect(tester.getSize(_findFirstContainerFor('Data')).height, localThemeDataRowHeight); + }); } + BoxDecoration _tableRowBoxDecoration({required WidgetTester tester, required int index}) { final Table table = tester.widget(find.byType(Table)); final TableRow tableRow = table.children[index]; diff --git a/packages/flutter/test/material/paginated_data_table_test.dart b/packages/flutter/test/material/paginated_data_table_test.dart index 4f97d771c8..0e72cbc4ab 100644 --- a/packages/flutter/test/material/paginated_data_table_test.dart +++ b/packages/flutter/test/material/paginated_data_table_test.dart @@ -405,7 +405,9 @@ void main() { final TestDataSource source = TestDataSource(); Widget buildCustomHeightPaginatedTable({ - double dataRowHeight = 48.0, + double? dataRowHeight, + double? dataRowMinHeight, + double? dataRowMaxHeight, double headingRowHeight = 56.0, }) { return PaginatedDataTable( @@ -423,6 +425,8 @@ void main() { DataColumn(label: Text('Generation')), ], dataRowHeight: dataRowHeight, + dataRowMinHeight: dataRowMinHeight, + dataRowMaxHeight: dataRowMaxHeight, headingRowHeight: headingRowHeight, ); } @@ -480,6 +484,13 @@ void main() { expect(tester.renderObject( find.widgetWithText(Container, 'Frozen yogurt (0)').first, ).size.height, 56.0); + + await tester.pumpWidget(MaterialApp( + home: Material(child: buildCustomHeightPaginatedTable(dataRowMinHeight: 51.0, dataRowMaxHeight: 51.0)), + )); + expect(tester.renderObject( + find.widgetWithText(Container, 'Frozen yogurt (0)').first, + ).size.height, 51.0); }); testWidgets('PaginatedDataTable custom horizontal padding - checkbox', (WidgetTester tester) async {