ListTile Material Ripple and Shape Patch (#73618)
This PR replaces the ColoredBox that ListTile uses with an Ink widget. That Ink widget is given a ShapeDecoration with the ListTile's color and shape. This fixes issues where the ListTile color would obscure material ripple effects, and cause the specified shape to not be respected.
List which issues are fixed by this PR. You must list at least one issue.
Fixes #73616
Fixes #63877
Fixes #67117
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
I modified a handful of tests related to ListTiles. The tests I changed had implementation-specific ways of checking the ListTile color. I have rewritten those so that instead of checking for a ColoredBox with a specific color, they check that a path is painted with the correct color.
I added the following tests to list_tile_test.dart:
"ListTile shows Material ripple effects on top of tileColor" (Regression test for #73616)
"ListTile shape is painted correctly" (Regression test for #63877)
I added the following test to sliver_prototype_item_extent_test.dart:
"SliverPrototypeExtentList prototypeItem paint transform is zero" (Regression test for #67117)
diff --git a/packages/flutter/lib/src/material/list_tile.dart b/packages/flutter/lib/src/material/list_tile.dart
index b3acb8c..04ac50d 100644
--- a/packages/flutter/lib/src/material/list_tile.dart
+++ b/packages/flutter/lib/src/material/list_tile.dart
@@ -12,6 +12,7 @@
import 'constants.dart';
import 'debug.dart';
import 'divider.dart';
+import 'ink_decoration.dart';
import 'ink_well.dart';
import 'material_state.dart';
import 'theme.dart';
@@ -109,7 +110,7 @@
final bool dense;
/// {@template flutter.material.ListTileTheme.shape}
- /// If specified, [shape] defines the shape of the [ListTile]'s [InkWell] border.
+ /// If specified, [shape] defines the [ListTile]'s shape.
/// {@endtemplate}
final ShapeBorder? shape;
@@ -838,13 +839,12 @@
/// widgets within a [Theme].
final VisualDensity? visualDensity;
- /// The shape of the tile's [InkWell].
+ /// The tile's shape.
///
- /// Defines the tile's [InkWell.customBorder].
+ /// Defines the tile's [InkWell.customBorder] and [Ink.decoration] shape.
///
- /// If this property is null then [CardTheme.shape] of [ThemeData.cardTheme]
- /// is used. If that's null then the shape will be a [RoundedRectangleBorder]
- /// with a circular corner radius of 4.0.
+ /// If this property is null then [ListTileTheme.shape] is used.
+ /// If that's null then a rectangular [Border] will be used.
final ShapeBorder? shape;
/// The tile's internal padding.
@@ -1183,8 +1183,11 @@
child: Semantics(
selected: selected,
enabled: enabled,
- child: ColoredBox(
- color: _tileBackgroundColor(tileTheme),
+ child: Ink(
+ decoration: ShapeDecoration(
+ shape: shape ?? tileTheme.shape ?? const Border(),
+ color: _tileBackgroundColor(tileTheme),
+ ),
child: SafeArea(
top: false,
bottom: false,
diff --git a/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart b/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart
index 7bb575a..f0dfe0c 100644
--- a/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart
+++ b/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart
@@ -577,7 +577,13 @@
@override
void applyPaintTransform(RenderBox child, Matrix4 transform) {
- if (_keepAliveBucket.containsKey(indexOf(child))) {
+ final SliverMultiBoxAdaptorParentData childParentData = child.parentData! as SliverMultiBoxAdaptorParentData;
+ if (childParentData.index == null) {
+ // If the child has no index, such as with the prototype of a
+ // SliverPrototypeExtentList, then it is not visible, so we give it a
+ // zero transform to prevent it from painting.
+ transform.setZero();
+ } else if (_keepAliveBucket.containsKey(childParentData.index)) {
// It is possible that widgets under kept alive children want to paint
// themselves. For example, the Material widget tries to paint all
// InkFeatures under its subtree as long as they are not disposed. In
diff --git a/packages/flutter/test/material/checkbox_list_tile_test.dart b/packages/flutter/test/material/checkbox_list_tile_test.dart
index 28429fd..5e13e50 100644
--- a/packages/flutter/test/material/checkbox_list_tile_test.dart
+++ b/packages/flutter/test/material/checkbox_list_tile_test.dart
@@ -129,7 +129,7 @@
)
);
- final Rect paddingRect = tester.getRect(find.byType(Padding));
+ final Rect paddingRect = tester.getRect(find.byType(SafeArea));
final Rect checkboxRect = tester.getRect(find.byType(Checkbox));
final Rect titleRect = tester.getRect(find.text('Title'));
@@ -241,35 +241,34 @@
});
testWidgets('CheckboxListTile respects tileColor', (WidgetTester tester) async {
- const Color tileColor = Colors.black;
+ final Color tileColor = Colors.red.shade500;
await tester.pumpWidget(
wrap(
- child: const Center(
+ child: Center(
child: CheckboxListTile(
value: false,
onChanged: null,
- title: Text('Title'),
+ title: const Text('Title'),
tileColor: tileColor,
),
),
),
);
- final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox));
- expect(coloredBox.color, equals(tileColor));
+ expect(find.byType(Material), paints..path(color: tileColor));
});
testWidgets('CheckboxListTile respects selectedTileColor', (WidgetTester tester) async {
- const Color selectedTileColor = Colors.black;
+ final Color selectedTileColor = Colors.green.shade500;
await tester.pumpWidget(
wrap(
- child: const Center(
+ child: Center(
child: CheckboxListTile(
value: false,
onChanged: null,
- title: Text('Title'),
+ title: const Text('Title'),
selected: true,
selectedTileColor: selectedTileColor,
),
@@ -277,7 +276,6 @@
),
);
- final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox));
- expect(coloredBox.color, equals(selectedTileColor));
+ expect(find.byType(Material), paints..path(color: selectedTileColor));
});
}
diff --git a/packages/flutter/test/material/list_tile_test.dart b/packages/flutter/test/material/list_tile_test.dart
index 777a072..4308e63 100644
--- a/packages/flutter/test/material/list_tile_test.dart
+++ b/packages/flutter/test/material/list_tile_test.dart
@@ -1250,7 +1250,6 @@
testWidgets('ListTile is focusable and has correct focus color', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode(debugLabel: 'ListTile');
tester.binding.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
- const Key tileKey = Key('listTile');
Widget buildApp({bool enabled = true}) {
return MaterialApp(
home: Material(
@@ -1261,7 +1260,6 @@
height: 100,
color: Colors.white,
child: ListTile(
- key: tileKey,
onTap: enabled ? () {} : null,
focusColor: Colors.orange[500],
autofocus: true,
@@ -1278,16 +1276,14 @@
await tester.pumpAndSettle();
expect(focusNode.hasPrimaryFocus, isTrue);
expect(
- Material.of(tester.element(find.byKey(tileKey))),
+ find.byType(Material),
paints
..rect(
- color: Colors.orange[500],
- rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0),
- )
+ color: Colors.orange[500],
+ rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0))
..rect(
- color: const Color(0xffffffff),
- rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0),
- ),
+ color: const Color(0xffffffff),
+ rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0)),
);
// Check when the list tile is disabled.
@@ -1295,7 +1291,7 @@
await tester.pumpAndSettle();
expect(focusNode.hasPrimaryFocus, isFalse);
expect(
- Material.of(tester.element(find.byKey(tileKey))),
+ find.byType(Material),
paints
..rect(
color: const Color(0xffffffff),
@@ -1305,7 +1301,6 @@
testWidgets('ListTile can be hovered and has correct hover color', (WidgetTester tester) async {
tester.binding.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
- const Key tileKey = Key('ListTile');
Widget buildApp({bool enabled = true}) {
return MaterialApp(
home: Material(
@@ -1316,7 +1311,6 @@
height: 100,
color: Colors.white,
child: ListTile(
- key: tileKey,
onTap: enabled ? () {} : null,
hoverColor: Colors.orange[500],
autofocus: true,
@@ -1332,7 +1326,7 @@
await tester.pump();
await tester.pumpAndSettle();
expect(
- Material.of(tester.element(find.byKey(tileKey))),
+ find.byType(Material),
paints
..rect(
color: const Color(0x1f000000),
@@ -1345,30 +1339,30 @@
// Start hovering
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
- await gesture.moveTo(tester.getCenter(find.byKey(tileKey)));
+ await gesture.moveTo(tester.getCenter(find.byType(ListTile)));
await tester.pumpWidget(buildApp());
await tester.pump();
await tester.pumpAndSettle();
expect(
- Material.of(tester.element(find.byKey(tileKey))),
- paints
- ..rect(
- color: const Color(0x1f000000),
- rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0))
- ..rect(
- color: Colors.orange[500],
- rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0))
- ..rect(
- color: const Color(0xffffffff),
- rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0)),
+ find.byType(Material),
+ paints
+ ..rect(
+ color: const Color(0x1f000000),
+ rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0))
+ ..rect(
+ color: Colors.orange[500],
+ rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0))
+ ..rect(
+ color: const Color(0xffffffff),
+ rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0)),
);
await tester.pumpWidget(buildApp(enabled: false));
await tester.pump();
await tester.pumpAndSettle();
expect(
- Material.of(tester.element(find.byKey(tileKey))),
+ find.byType(Material),
paints
..rect(
color: Colors.orange[500],
@@ -1455,6 +1449,75 @@
expect(box.size, equals(const Size(800, 44)));
});
+ testWidgets('ListTile shape is painted correctly', (WidgetTester tester) async {
+ // Regression test for https://github.com/flutter/flutter/issues/63877
+ const ShapeBorder rectShape = RoundedRectangleBorder();
+ const ShapeBorder stadiumShape = StadiumBorder();
+ final Color tileColor = Colors.red.shade500;
+
+ Widget buildListTile(ShapeBorder shape) {
+ return MaterialApp(
+ home: Material(
+ child: Center(
+ child: ListTile(shape: shape, tileColor: tileColor),
+ ),
+ ),
+ );
+ }
+
+ // Test rectangle shape
+ await tester.pumpWidget(buildListTile(rectShape));
+ Rect rect = tester.getRect(find.byType(ListTile));
+
+ // Check if a path was painted with the correct color and shape
+ expect(
+ find.byType(Material),
+ paints..path(
+ color: tileColor,
+ // Corners should be included
+ includes: <Offset>[
+ Offset(rect.left, rect.top),
+ Offset(rect.right, rect.top),
+ Offset(rect.left, rect.bottom),
+ Offset(rect.right, rect.bottom),
+ ],
+ // Points outside rect should be excluded
+ excludes: <Offset>[
+ Offset(rect.left - 1, rect.top - 1),
+ Offset(rect.right + 1, rect.top - 1),
+ Offset(rect.left - 1, rect.bottom + 1),
+ Offset(rect.right + 1, rect.bottom + 1),
+ ],
+ ),
+ );
+
+ // Test stadium shape
+ await tester.pumpWidget(buildListTile(stadiumShape));
+ rect = tester.getRect(find.byType(ListTile));
+
+ // Check if a path was painted with the correct color and shape
+ expect(
+ find.byType(Material),
+ paints..path(
+ color: tileColor,
+ // Center points of sides should be included
+ includes: <Offset>[
+ Offset(rect.left + rect.width / 2, rect.top),
+ Offset(rect.left, rect.top + rect.height / 2),
+ Offset(rect.right, rect.top + rect.height / 2),
+ Offset(rect.left + rect.width / 2, rect.bottom),
+ ],
+ // Corners should be excluded
+ excludes: <Offset>[
+ Offset(rect.left, rect.top),
+ Offset(rect.right, rect.top),
+ Offset(rect.left, rect.bottom),
+ Offset(rect.right, rect.bottom),
+ ],
+ ),
+ );
+ });
+
testWidgets('ListTile changes mouse cursor when hovered', (WidgetTester tester) async {
// Test ListTile() constructor
await tester.pumpWidget(
@@ -1536,8 +1599,8 @@
testWidgets('ListTile respects tileColor & selectedTileColor', (WidgetTester tester) async {
bool isSelected = false;
- const Color selectedTileColor = Colors.red;
- const Color tileColor = Colors.green;
+ final Color tileColor = Colors.green.shade500;
+ final Color selectedTileColor = Colors.red.shade500;
await tester.pumpWidget(
MaterialApp(
@@ -1562,16 +1625,48 @@
);
// Initially, when isSelected is false, the ListTile should respect tileColor.
- ColoredBox coloredBox = tester.widget(find.byType(ColoredBox));
- expect(coloredBox.color, tileColor);
+ expect(find.byType(Material), paints..path(color: tileColor));
// Tap on tile to change isSelected.
await tester.tap(find.byType(ListTile));
await tester.pumpAndSettle();
// When isSelected is true, the ListTile should respect selectedTileColor.
- coloredBox = tester.widget(find.byType(ColoredBox));
- expect(coloredBox.color, selectedTileColor);
+ expect(find.byType(Material), paints..path(color: selectedTileColor));
+ });
+
+ testWidgets('ListTile shows Material ripple effects on top of tileColor', (WidgetTester tester) async {
+ // Regression test for https://github.com/flutter/flutter/issues/73616
+ final Color tileColor = Colors.red.shade500;
+
+ await tester.pumpWidget(
+ MaterialApp(
+ home: Material(
+ child: Center(
+ child: ListTile(
+ tileColor: tileColor,
+ onTap: () {},
+ title: const Text('Title'),
+ ),
+ ),
+ ),
+ ),
+ );
+
+ // Before ListTile is tapped, it should be tileColor
+ expect(find.byType(Material), paints..path(color: tileColor));
+
+ // Tap on tile to trigger ink effect and wait for it to be underway.
+ await tester.tap(find.byType(ListTile));
+ await tester.pump(const Duration(milliseconds: 200));
+
+ // After tap, the tile could be drawn in tileColor, with the ripple (circle) on top
+ expect(
+ find.byType(Material),
+ paints
+ ..path(color: tileColor)
+ ..circle(),
+ );
});
testWidgets('ListTile default tile color', (WidgetTester tester) async {
@@ -1598,16 +1693,13 @@
),
);
- ColoredBox coloredBox = tester.widget(find.byType(ColoredBox));
- expect(coloredBox.color, defaultColor);
+ expect(find.byType(Material), paints..path(color: defaultColor));
// Tap on tile to change isSelected.
await tester.tap(find.byType(ListTile));
await tester.pumpAndSettle();
- coloredBox = tester.widget(find.byType(ColoredBox));
- expect(isSelected, isTrue);
- expect(coloredBox.color, defaultColor);
+ expect(find.byType(Material), paints..path(color: defaultColor));
});
testWidgets('ListTile respects ListTileTheme\'s tileColor & selectedTileColor', (WidgetTester tester) async {
@@ -1618,8 +1710,8 @@
MaterialApp(
home: Material(
child: ListTileTheme(
- selectedTileColor: Colors.green,
- tileColor: Colors.red,
+ tileColor: const Color(0xff00ff00),
+ selectedTileColor: const Color(0xffff0000),
child: Center(
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
@@ -1639,21 +1731,19 @@
),
);
- ColoredBox coloredBox = tester.widget(find.byType(ColoredBox));
- expect(coloredBox.color, theme.tileColor);
+ expect(find.byType(Material), paints..path(color: theme.tileColor));
// Tap on tile to change isSelected.
await tester.tap(find.byType(ListTile));
await tester.pumpAndSettle();
- coloredBox = tester.widget(find.byType(ColoredBox));
- expect(coloredBox.color, theme.selectedTileColor);
+ expect(find.byType(Material), paints..path(color: theme.selectedTileColor));
});
testWidgets('ListTileTheme\'s tileColor & selectedTileColor are overridden by ListTile properties', (WidgetTester tester) async {
bool isSelected = false;
- const Color tileColor = Colors.brown;
- const Color selectedTileColor = Colors.purple;
+ final Color tileColor = Colors.green.shade500;
+ final Color selectedTileColor = Colors.red.shade500;
await tester.pumpWidget(
MaterialApp(
@@ -1681,15 +1771,13 @@
),
);
- ColoredBox coloredBox = tester.widget(find.byType(ColoredBox));
- expect(coloredBox.color, tileColor);
+ expect(find.byType(Material), paints..path(color: tileColor));
// Tap on tile to change isSelected.
await tester.tap(find.byType(ListTile));
await tester.pumpAndSettle();
- coloredBox = tester.widget(find.byType(ColoredBox));
- expect(coloredBox.color, selectedTileColor);
+ expect(find.byType(Material), paints..path(color: selectedTileColor));
});
testWidgets('ListTile layout at zero size', (WidgetTester tester) async {
diff --git a/packages/flutter/test/material/radio_list_tile_test.dart b/packages/flutter/test/material/radio_list_tile_test.dart
index 704cb63..3def2d7 100644
--- a/packages/flutter/test/material/radio_list_tile_test.dart
+++ b/packages/flutter/test/material/radio_list_tile_test.dart
@@ -9,6 +9,7 @@
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/material.dart';
+import '../rendering/mock_canvas.dart';
import '../widgets/semantics_tester.dart';
@@ -628,7 +629,7 @@
)
);
- final Rect paddingRect = tester.getRect(find.byType(Padding));
+ final Rect paddingRect = tester.getRect(find.byType(SafeArea));
final Rect radioRect = tester.getRect(find.byType(radioType));
final Rect titleRect = tester.getRect(find.text('Title'));
@@ -668,37 +669,36 @@
});
testWidgets('RadioListTile respects tileColor', (WidgetTester tester) async {
- const Color tileColor = Colors.red;
+ final Color tileColor = Colors.red.shade500;
await tester.pumpWidget(
wrap(
- child: const Center(
+ child: Center(
child: RadioListTile<bool>(
value: false,
groupValue: true,
onChanged: null,
- title: Text('Title'),
+ title: const Text('Title'),
tileColor: tileColor,
),
),
),
);
- final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox));
- expect(coloredBox.color, tileColor);
+ expect(find.byType(Material), paints..path(color: tileColor));
});
testWidgets('RadioListTile respects selectedTileColor', (WidgetTester tester) async {
- const Color selectedTileColor = Colors.black;
+ final Color selectedTileColor = Colors.green.shade500;
await tester.pumpWidget(
wrap(
- child: const Center(
+ child: Center(
child: RadioListTile<bool>(
value: false,
groupValue: true,
onChanged: null,
- title: Text('Title'),
+ title: const Text('Title'),
selected: true,
selectedTileColor: selectedTileColor,
),
@@ -706,7 +706,6 @@
),
);
- final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox));
- expect(coloredBox.color, equals(selectedTileColor));
+ expect(find.byType(Material), paints..path(color: selectedTileColor));
});
}
diff --git a/packages/flutter/test/material/switch_list_tile_test.dart b/packages/flutter/test/material/switch_list_tile_test.dart
index 4c7a3c0..eb03860 100644
--- a/packages/flutter/test/material/switch_list_tile_test.dart
+++ b/packages/flutter/test/material/switch_list_tile_test.dart
@@ -359,35 +359,34 @@
});
testWidgets('SwitchListTile respects tileColor', (WidgetTester tester) async {
- const Color tileColor = Colors.red;
+ final Color tileColor = Colors.red.shade500;
await tester.pumpWidget(
wrap(
- child: const Center(
+ child: Center(
child: SwitchListTile(
value: false,
onChanged: null,
- title: Text('Title'),
+ title: const Text('Title'),
tileColor: tileColor,
),
),
),
);
- final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox));
- expect(coloredBox.color, tileColor);
+ expect(find.byType(Material), paints..path(color: tileColor));
});
testWidgets('SwitchListTile respects selectedTileColor', (WidgetTester tester) async {
- const Color selectedTileColor = Colors.black;
+ final Color selectedTileColor = Colors.green.shade500;
await tester.pumpWidget(
wrap(
- child: const Center(
+ child: Center(
child: SwitchListTile(
value: false,
onChanged: null,
- title: Text('Title'),
+ title: const Text('Title'),
selected: true,
selectedTileColor: selectedTileColor,
),
@@ -395,8 +394,7 @@
),
);
- final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox));
- expect(coloredBox.color, equals(selectedTileColor));
+ expect(find.byType(Material), paints..path(color: selectedTileColor));
});
}
diff --git a/packages/flutter/test/widgets/sliver_prototype_item_extent_test.dart b/packages/flutter/test/widgets/sliver_prototype_item_extent_test.dart
index ad513be..3a4b0a0 100644
--- a/packages/flutter/test/widgets/sliver_prototype_item_extent_test.dart
+++ b/packages/flutter/test/widgets/sliver_prototype_item_extent_test.dart
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/widgets.dart';
@@ -21,14 +22,14 @@
}
}
-Widget buildFrame({ int? count, double? width, double? height, Axis? scrollDirection }) {
+Widget buildFrame({ int? count, double? width, double? height, Axis? scrollDirection, Key? prototypeKey }) {
return Directionality(
textDirection: TextDirection.ltr,
child: CustomScrollView(
scrollDirection: scrollDirection ?? Axis.vertical,
slivers: <Widget>[
SliverPrototypeExtentList(
- prototypeItem: TestItem(item: -1, width: width, height: height),
+ prototypeItem: TestItem(item: -1, width: width, height: height, key: prototypeKey),
delegate: SliverChildBuilderDelegate(
(BuildContext context, int index) => TestItem(item: index),
childCount: count,
@@ -136,4 +137,18 @@
for (int i = 1; i < 10; i += 1)
expect(find.text('Item $i'), findsOneWidget);
});
+
+ testWidgets('SliverPrototypeExtentList prototypeItem paint transform is zero.', (WidgetTester tester) async {
+ // Regression test for https://github.com/flutter/flutter/issues/67117
+ // This test ensures that the SliverPrototypeExtentList does not cause an
+ // assertion error when calculating the paint transform of its prototypeItem.
+ // The paint transform of the prototypeItem should be zero, since it is not visible.
+ final GlobalKey prototypeKey = GlobalKey();
+ await tester.pumpWidget(buildFrame(count: 20, height: 100.0, prototypeKey: prototypeKey));
+
+ final RenderObject scrollView = tester.renderObject(find.byType(CustomScrollView));
+ final RenderObject prototype = prototypeKey.currentContext!.findRenderObject()!;
+
+ expect(prototype.getTransformTo(scrollView), Matrix4.zero());
+ });
}
