Unverified Commit 78ff7707 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Fix niggling PaginatedDataTable bugs (#13556)

Prevent header from thinking it can wrap and then overflowing.

Fix default footer string which lost its colon (localized values are fine).

Make the "rows per page" drop-down include at least one value even when the table lacks many items. (Previously it would assert if your table was too short.)

Make the footer scrollable.

Fix some todos and improve some debug output.

Tests for much of the above.
parent b5eac042
......@@ -109,6 +109,13 @@ List<GalleryItem> _buildGalleryItems() {
routeName: ChipDemo.routeName,
buildRoute: (BuildContext context) => new ChipDemo(),
),
new GalleryItem(
title: 'Data tables',
subtitle: 'Data tables',
category: 'Material Components',
routeName: DataTableDemo.routeName,
buildRoute: (BuildContext context) => new DataTableDemo(),
),
new GalleryItem(
title: 'Date and time pickers',
subtitle: 'Date and time selection widgets',
......
......@@ -411,14 +411,15 @@ class DataTable extends StatelessWidget {
alignment: numeric ? Alignment.centerRight : AlignmentDirectional.centerStart,
child: new AnimatedDefaultTextStyle(
style: new TextStyle(
// TODO(ianh): font family should be Roboto; see https://github.com/flutter/flutter/issues/3116
// TODO(ianh): font family should match Theme; see https://github.com/flutter/flutter/issues/3116
fontWeight: FontWeight.w500,
fontSize: _kHeadingFontSize,
height: _kHeadingRowHeight / _kHeadingFontSize,
height: math.min(1.0, _kHeadingRowHeight / _kHeadingFontSize),
color: (Theme.of(context).brightness == Brightness.light)
? ((onSort != null && sorted) ? Colors.black87 : Colors.black54)
: ((onSort != null && sorted) ? Colors.white : Colors.white70),
),
softWrap: false,
duration: _kSortArrowAnimationDuration,
child: label,
),
......
......@@ -503,7 +503,7 @@ class DefaultMaterialLocalizations implements MaterialLocalizations {
}
@override
String get rowsPerPageTitle => 'Rows per page';
String get rowsPerPageTitle => 'Rows per page:';
@override
String selectedRowCountTitle(int selectedRowCount) {
......
......@@ -324,7 +324,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
final List<Widget> footerWidgets = <Widget>[];
if (widget.onRowsPerPageChanged != null) {
final List<Widget> availableRowsPerPage = widget.availableRowsPerPage
.where((int value) => value <= _rowCount)
.where((int value) => (value <= _rowCount || value == widget.rowsPerPage))
.map<DropdownMenuItem<int>>((int value) {
return new DropdownMenuItem<int>(
value: value,
......@@ -333,15 +333,22 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
})
.toList();
footerWidgets.addAll(<Widget>[
new Container(width: 14.0), // to match trailing padding in case we overflow and end up scrolling
new Text(localizations.rowsPerPageTitle),
new DropdownButtonHideUnderline(
child: new DropdownButton<int>(
items: availableRowsPerPage,
value: widget.rowsPerPage,
onChanged: widget.onRowsPerPageChanged,
style: footerTextStyle,
iconSize: 24.0
)
new ConstrainedBox(
constraints: const BoxConstraints(minWidth: 64.0), // 40.0 for the text, 24.0 for the icon
child: new Align(
alignment: AlignmentDirectional.centerEnd,
child: new DropdownButtonHideUnderline(
child: new DropdownButton<int>(
items: availableRowsPerPage,
value: widget.rowsPerPage,
onChanged: widget.onRowsPerPageChanged,
style: footerTextStyle,
iconSize: 24.0,
),
),
),
),
]);
}
......@@ -422,15 +429,18 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
),
child: new Container(
height: 56.0,
child: new Row(
mainAxisAlignment: MainAxisAlignment.end,
children: footerWidgets
)
)
)
)
]
)
child: new SingleChildScrollView(
scrollDirection: Axis.horizontal,
reverse: true,
child: new Row(
children: footerWidgets,
),
),
),
),
),
],
),
);
}
}
......@@ -407,7 +407,7 @@ class TextPainter {
final int nextCodeUnit = _text.codeUnitAt(offset);
if (nextCodeUnit == null)
return null;
// TODO(goderbauer): doesn't handle flag emojis (https://github.com/flutter/flutter/issues/13404).
// TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404).
return _isUtf16Surrogate(nextCodeUnit) ? offset + 2 : offset + 1;
}
......@@ -417,7 +417,7 @@ class TextPainter {
final int prevCodeUnit = _text.codeUnitAt(offset - 1);
if (prevCodeUnit == null)
return null;
// TODO(goderbauer): doesn't handle flag emojis (https://github.com/flutter/flutter/issues/13404).
// TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404).
return _isUtf16Surrogate(prevCodeUnit) ? offset - 2 : offset - 1;
}
......
......@@ -113,6 +113,9 @@ class IntrinsicColumnWidth extends TableColumnWidth {
@override
double flex(Iterable<RenderBox> cells) => _flex;
@override
String toString() => '$runtimeType(flex: ${_flex?.toStringAsFixed(1)})';
}
/// Sizes the column to a specific number of pixels.
......
......@@ -144,6 +144,10 @@ class DefaultTextStyle extends InheritedWidget {
void debugFillProperties(DiagnosticPropertiesBuilder description) {
super.debugFillProperties(description);
style?.debugFillProperties(description);
description.add(new EnumProperty<TextAlign>('textAlign', textAlign, defaultValue: null));
description.add(new FlagProperty('softWrap', value: softWrap, ifTrue: 'wrapping at box width', ifFalse: 'no wrapping except at line break characters', showName: true));
description.add(new EnumProperty<TextOverflow>('overflow', overflow, defaultValue: null));
description.add(new IntProperty('maxLines', maxLines, defaultValue: null));
}
}
......
......@@ -98,4 +98,116 @@ void main() {
expect(log, <String>['row-selected: KitKat']);
log.clear();
});
testWidgets('DataTable overflow test - header', (WidgetTester tester) async {
await tester.pumpWidget(
new MaterialApp(
home: new Material(
child: new DataTable(
columns: <DataColumn>[
new DataColumn(
label: new Text('X' * 2000),
),
],
rows: const <DataRow>[
const DataRow(
cells: const <DataCell>[
const DataCell(
const Text('X'),
),
],
),
],
),
),
),
);
expect(tester.renderObject<RenderBox>(find.byType(Text).first).size.width, greaterThan(800.0));
expect(tester.renderObject<RenderBox>(find.byType(Row).first).size.width, greaterThan(800.0));
expect(tester.takeException(), isNull); // column overflows table, but text doesn't overflow cell
});
testWidgets('DataTable overflow test - header with spaces', (WidgetTester tester) async {
await tester.pumpWidget(
new MaterialApp(
home: new Material(
child: new DataTable(
columns: <DataColumn>[
new DataColumn(
label: new Text('X ' * 2000), // has soft wrap points, but they should be ignored
),
],
rows: const <DataRow>[
const DataRow(
cells: const <DataCell>[
const DataCell(
const Text('X'),
),
],
),
],
),
),
),
);
expect(tester.renderObject<RenderBox>(find.byType(Text).first).size.width, greaterThan(800.0));
expect(tester.renderObject<RenderBox>(find.byType(Row).first).size.width, greaterThan(800.0));
expect(tester.takeException(), isNull); // column overflows table, but text doesn't overflow cell
}, skip: true); // https://github.com/flutter/flutter/issues/13512
testWidgets('DataTable overflow test', (WidgetTester tester) async {
await tester.pumpWidget(
new MaterialApp(
home: new Material(
child: new DataTable(
columns: const <DataColumn>[
const DataColumn(
label: const Text('X'),
),
],
rows: <DataRow>[
new DataRow(
cells: <DataCell>[
new DataCell(
new Text('X' * 2000),
),
],
),
],
),
),
),
);
expect(tester.renderObject<RenderBox>(find.byType(Text).first).size.width, lessThan(800.0));
expect(tester.renderObject<RenderBox>(find.byType(Row).first).size.width, greaterThan(800.0));
expect(tester.takeException(), isNull); // cell overflows table, but text doesn't overflow cell
});
testWidgets('DataTable overflow test', (WidgetTester tester) async {
await tester.pumpWidget(
new MaterialApp(
home: new Material(
child: new DataTable(
columns: const <DataColumn>[
const DataColumn(
label: const Text('X'),
),
],
rows: <DataRow>[
new DataRow(
cells: <DataCell>[
new DataCell(
new Text('X ' * 2000), // wraps
),
],
),
],
),
),
),
);
expect(tester.renderObject<RenderBox>(find.byType(Text).first).size.width, lessThan(800.0));
expect(tester.renderObject<RenderBox>(find.byType(Row).first).size.width, lessThan(800.0));
expect(tester.takeException(), isNull);
});
}
......@@ -192,4 +192,90 @@ void main() {
expect(log, <String>['action: adjust']);
log.clear();
});
testWidgets('PaginatedDataTable text alignment', (WidgetTester tester) async {
await tester.pumpWidget(new MaterialApp(
home: new PaginatedDataTable(
header: const Text('HEADER'),
source: new TestDataSource(),
rowsPerPage: 8,
availableRowsPerPage: <int>[
8, 9,
],
onRowsPerPageChanged: (int rowsPerPage) { },
columns: <DataColumn>[
const DataColumn(label: const Text('COL1')),
const DataColumn(label: const Text('COL2')),
const DataColumn(label: const Text('COL3')),
],
),
));
expect(find.text('Rows per page:'), findsOneWidget);
expect(find.text('8'), findsOneWidget);
expect(tester.getTopRight(find.text('8')).dx, tester.getTopRight(find.text('Rows per page:')).dx + 40.0); // per spec
});
testWidgets('PaginatedDataTable with large text', (WidgetTester tester) async {
final TestDataSource source = new TestDataSource();
await tester.pumpWidget(new MaterialApp(
home: new MediaQuery(
data: const MediaQueryData(
textScaleFactor: 20.0,
),
child: new PaginatedDataTable(
header: const Text('HEADER'),
source: source,
rowsPerPage: 501,
availableRowsPerPage: <int>[ 501 ],
onRowsPerPageChanged: (int rowsPerPage) { },
columns: <DataColumn>[
const DataColumn(label: const Text('COL1')),
const DataColumn(label: const Text('COL2')),
const DataColumn(label: const Text('COL3')),
],
),
),
));
// the column overflows because we're forcing it to 600 pixels high
expect(tester.takeException(), contains('A RenderFlex overflowed by'));
expect(find.text('Rows per page:'), findsOneWidget);
// Test that we will show some options in the drop down even if the lowest option is bigger than the source:
assert(501 > source.rowCount);
expect(find.text('501'), findsOneWidget);
// Test that it fits:
expect(tester.getTopRight(find.text('501')).dx, greaterThanOrEqualTo(tester.getTopRight(find.text('Rows per page:')).dx + 40.0));
});
testWidgets('PaginatedDataTable footer scrolls', (WidgetTester tester) async {
final TestDataSource source = new TestDataSource();
await tester.pumpWidget(new MaterialApp(
home: new Align(
alignment: Alignment.topLeft,
child: new SizedBox(
width: 100.0,
child: new PaginatedDataTable(
header: const Text('HEADER'),
source: source,
rowsPerPage: 5,
availableRowsPerPage: <int>[ 5 ],
onRowsPerPageChanged: (int rowsPerPage) { },
columns: <DataColumn>[
const DataColumn(label: const Text('COL1')),
const DataColumn(label: const Text('COL2')),
const DataColumn(label: const Text('COL3')),
],
),
),
),
));
expect(find.text('Rows per page:'), findsOneWidget);
expect(tester.getTopLeft(find.text('Rows per page:')).dx, lessThan(0.0)); // off screen
await tester.dragFrom(
new Offset(50.0, tester.getTopLeft(find.text('Rows per page:')).dy),
const Offset(1000.0, 0.0),
);
await tester.pump();
expect(find.text('Rows per page:'), findsOneWidget);
expect(tester.getTopLeft(find.text('Rows per page:')).dx, 18.0); // 14 padding in the footer row, 4 padding from the card
});
}
......@@ -82,7 +82,7 @@ void main() {
' │ parentData: offset=Offset(335.0, 185.0) (can use size)\n'
' │ constraints: BoxConstraints(0.0<=w<=800.0, 0.0<=h<=600.0)\n'
' │ size: Size(130.0, 230.0)\n'
' │ default column width: IntrinsicColumnWidth\n'
' │ default column width: IntrinsicColumnWidth(flex: null)\n'
' │ table size: 5×5\n'
' │ column offsets: 0.0, 10.0, 30.0, 130.0, 130.0\n'
' │ row offsets: 0.0, 30.0, 30.0, 30.0, 30.0, 230.0\n'
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment