Unverified Commit 88118bcb authored by Wikiwix's avatar Wikiwix Committed by GitHub

Fix sort indicator for DataTables (#62795)

* Fix sort indicator for DataTables

As per material spec ascending order should be shown via an upward arrow
This commit changes the displayed arrows accordingly.

* Test sort indicator orientation in DataTable
parent 3ff76f47
...@@ -610,7 +610,7 @@ class DataTable extends StatelessWidget { ...@@ -610,7 +610,7 @@ class DataTable extends StatelessWidget {
return onSort == null ? const <Widget>[] : <Widget>[ return onSort == null ? const <Widget>[] : <Widget>[
_SortArrow( _SortArrow(
visible: sorted, visible: sorted,
down: sorted ? ascending : null, up: sorted ? ascending : null,
duration: _sortArrowAnimationDuration, duration: _sortArrowAnimationDuration,
), ),
const SizedBox(width: _sortArrowPadding), const SizedBox(width: _sortArrowPadding),
...@@ -924,13 +924,13 @@ class _SortArrow extends StatefulWidget { ...@@ -924,13 +924,13 @@ class _SortArrow extends StatefulWidget {
const _SortArrow({ const _SortArrow({
Key key, Key key,
this.visible, this.visible,
this.down, this.up,
this.duration, this.duration,
}) : super(key: key); }) : super(key: key);
final bool visible; final bool visible;
final bool down; final bool up;
final Duration duration; final Duration duration;
...@@ -947,7 +947,7 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin { ...@@ -947,7 +947,7 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin {
Animation<double> _orientationAnimation; Animation<double> _orientationAnimation;
double _orientationOffset = 0.0; double _orientationOffset = 0.0;
bool _down; bool _up;
static final Animatable<double> _turnTween = Tween<double>(begin: 0.0, end: math.pi) static final Animatable<double> _turnTween = Tween<double>(begin: 0.0, end: math.pi)
.chain(CurveTween(curve: Curves.easeIn)); .chain(CurveTween(curve: Curves.easeIn));
...@@ -972,7 +972,7 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin { ...@@ -972,7 +972,7 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin {
..addListener(_rebuild) ..addListener(_rebuild)
..addStatusListener(_resetOrientationAnimation); ..addStatusListener(_resetOrientationAnimation);
if (widget.visible) if (widget.visible)
_orientationOffset = widget.down ? 0.0 : math.pi; _orientationOffset = widget.up ? 0.0 : math.pi;
} }
void _rebuild() { void _rebuild() {
...@@ -993,12 +993,12 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin { ...@@ -993,12 +993,12 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin {
void didUpdateWidget(_SortArrow oldWidget) { void didUpdateWidget(_SortArrow oldWidget) {
super.didUpdateWidget(oldWidget); super.didUpdateWidget(oldWidget);
bool skipArrow = false; bool skipArrow = false;
final bool newDown = widget.down ?? _down; final bool newUp = widget.up ?? _up;
if (oldWidget.visible != widget.visible) { if (oldWidget.visible != widget.visible) {
if (widget.visible && (_opacityController.status == AnimationStatus.dismissed)) { if (widget.visible && (_opacityController.status == AnimationStatus.dismissed)) {
_orientationController.stop(); _orientationController.stop();
_orientationController.value = 0.0; _orientationController.value = 0.0;
_orientationOffset = newDown ? 0.0 : math.pi; _orientationOffset = newUp ? 0.0 : math.pi;
skipArrow = true; skipArrow = true;
} }
if (widget.visible) { if (widget.visible) {
...@@ -1007,14 +1007,14 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin { ...@@ -1007,14 +1007,14 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin {
_opacityController.reverse(); _opacityController.reverse();
} }
} }
if ((_down != newDown) && !skipArrow) { if ((_up != newUp) && !skipArrow) {
if (_orientationController.status == AnimationStatus.dismissed) { if (_orientationController.status == AnimationStatus.dismissed) {
_orientationController.forward(); _orientationController.forward();
} else { } else {
_orientationController.reverse(); _orientationController.reverse();
} }
} }
_down = newDown; _up = newUp;
} }
@override @override
...@@ -1036,7 +1036,7 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin { ...@@ -1036,7 +1036,7 @@ class _SortArrowState extends State<_SortArrow> with TickerProviderStateMixin {
..setTranslationRaw(0.0, _arrowIconBaselineOffset, 0.0), ..setTranslationRaw(0.0, _arrowIconBaselineOffset, 0.0),
alignment: Alignment.center, alignment: Alignment.center,
child: Icon( child: Icon(
Icons.arrow_downward, Icons.arrow_upward,
size: _arrowIconSize, size: _arrowIconSize,
color: (Theme.of(context).brightness == Brightness.light) ? Colors.black87 : Colors.white70, color: (Theme.of(context).brightness == Brightness.light) ? Colors.black87 : Colors.white70,
), ),
......
...@@ -4,9 +4,12 @@ ...@@ -4,9 +4,12 @@
// @dart = 2.8 // @dart = 2.8
import 'dart:math' as math;
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:vector_math/vector_math_64.dart' show Matrix3;
import '../rendering/mock_canvas.dart'; import '../rendering/mock_canvas.dart';
import 'data_table_test_utils.dart'; import 'data_table_test_utils.dart';
...@@ -309,6 +312,54 @@ void main() { ...@@ -309,6 +312,54 @@ void main() {
expect(tester.takeException(), isNull); expect(tester.takeException(), isNull);
}); });
testWidgets('DataTable sort indicator orientation', (WidgetTester tester) async {
Widget buildTable({ bool sortAscending = true }) {
return DataTable(
sortColumnIndex: 0,
sortAscending: sortAscending,
columns: <DataColumn>[
DataColumn(
label: const Text('Name'),
tooltip: 'Name',
onSort: (int columnIndex, bool ascending) {},
),
],
rows: kDesserts.map<DataRow>((Dessert dessert) {
return DataRow(
cells: <DataCell>[
DataCell(
Text(dessert.name),
),
],
);
}).toList(),
);
}
// Check for ascending list
await tester.pumpWidget(MaterialApp(
home: Material(child: buildTable(sortAscending: true)),
));
// The `tester.widget` ensures that there is exactly one upward arrow.
Transform transformOfArrow = tester.widget<Transform>(find.widgetWithIcon(Transform, Icons.arrow_upward));
expect(
transformOfArrow.transform.getRotation(),
equals(Matrix3.identity())
);
// Check for descending list.
await tester.pumpWidget(MaterialApp(
home: Material(child: buildTable(sortAscending: false)),
));
await tester.pumpAndSettle();
// The `tester.widget` ensures that there is exactly one upward arrow.
transformOfArrow = tester.widget<Transform>(find.widgetWithIcon(Transform, Icons.arrow_upward));
expect(
transformOfArrow.transform.getRotation(),
equals(Matrix3.rotationZ(math.pi))
);
});
testWidgets('DataTable row onSelectChanged test', (WidgetTester tester) async { testWidgets('DataTable row onSelectChanged test', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
......
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