Unverified Commit 422916d2 authored by Nathan Walker's avatar Nathan Walker Committed by GitHub

ListTile Material Ripple and Shape Patch (#74373)

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.
parent ef849707
...@@ -215,13 +215,13 @@ class Ink extends StatefulWidget { ...@@ -215,13 +215,13 @@ class Ink extends StatefulWidget {
/// any [padding]. /// any [padding].
final double? height; final double? height;
EdgeInsetsGeometry? get _paddingIncludingDecoration { EdgeInsetsGeometry get _paddingIncludingDecoration {
if (decoration == null || decoration!.padding == null) if (decoration == null || decoration!.padding == null)
return padding; return padding ?? EdgeInsets.zero;
final EdgeInsetsGeometry? decorationPadding = decoration!.padding; final EdgeInsetsGeometry decorationPadding = decoration!.padding!;
if (padding == null) if (padding == null)
return decorationPadding; return decorationPadding;
return padding!.add(decorationPadding!); return padding!.add(decorationPadding);
} }
@override @override
...@@ -236,6 +236,7 @@ class Ink extends StatefulWidget { ...@@ -236,6 +236,7 @@ class Ink extends StatefulWidget {
} }
class _InkState extends State<Ink> { class _InkState extends State<Ink> {
final GlobalKey _boxKey = GlobalKey();
InkDecoration? _ink; InkDecoration? _ink;
void _handleRemoved() { void _handleRemoved() {
...@@ -249,31 +250,31 @@ class _InkState extends State<Ink> { ...@@ -249,31 +250,31 @@ class _InkState extends State<Ink> {
super.deactivate(); super.deactivate();
} }
Widget _build(BuildContext context, BoxConstraints constraints) { Widget _build(BuildContext context) {
// By creating the InkDecoration from within a Builder widget, we can
// use the RenderBox of the Padding widget.
if (_ink == null) { if (_ink == null) {
_ink = InkDecoration( _ink = InkDecoration(
decoration: widget.decoration, decoration: widget.decoration,
configuration: createLocalImageConfiguration(context), configuration: createLocalImageConfiguration(context),
controller: Material.of(context)!, controller: Material.of(context)!,
referenceBox: context.findRenderObject()! as RenderBox, referenceBox: _boxKey.currentContext!.findRenderObject()! as RenderBox,
onRemoved: _handleRemoved, onRemoved: _handleRemoved,
); );
} else { } else {
_ink!.decoration = widget.decoration; _ink!.decoration = widget.decoration;
_ink!.configuration = createLocalImageConfiguration(context); _ink!.configuration = createLocalImageConfiguration(context);
} }
Widget? current = widget.child; return widget.child ?? Container();
final EdgeInsetsGeometry? effectivePadding = widget._paddingIncludingDecoration;
if (effectivePadding != null)
current = Padding(padding: effectivePadding, child: current);
return current ?? Container();
} }
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
assert(debugCheckHasMaterial(context)); assert(debugCheckHasMaterial(context));
Widget result = LayoutBuilder( Widget result = Padding(
builder: _build, key: _boxKey,
padding: widget._paddingIncludingDecoration,
child: Builder(builder: _build),
); );
if (widget.width != null || widget.height != null) { if (widget.width != null || widget.height != null) {
result = SizedBox( result = SizedBox(
......
...@@ -11,6 +11,7 @@ import 'colors.dart'; ...@@ -11,6 +11,7 @@ import 'colors.dart';
import 'constants.dart'; import 'constants.dart';
import 'debug.dart'; import 'debug.dart';
import 'divider.dart'; import 'divider.dart';
import 'ink_decoration.dart';
import 'ink_well.dart'; import 'ink_well.dart';
import 'material_state.dart'; import 'material_state.dart';
import 'theme.dart'; import 'theme.dart';
...@@ -108,7 +109,7 @@ class ListTileTheme extends InheritedTheme { ...@@ -108,7 +109,7 @@ class ListTileTheme extends InheritedTheme {
final bool dense; final bool dense;
/// {@template flutter.material.ListTileTheme.shape} /// {@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} /// {@endtemplate}
final ShapeBorder? shape; final ShapeBorder? shape;
...@@ -837,13 +838,12 @@ class ListTile extends StatelessWidget { ...@@ -837,13 +838,12 @@ class ListTile extends StatelessWidget {
/// widgets within a [Theme]. /// widgets within a [Theme].
final VisualDensity? visualDensity; 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] /// If this property is null then [ListTileTheme.shape] is used.
/// is used. If that's null then the shape will be a [RoundedRectangleBorder] /// If that's null then a rectangular [Border] will be used.
/// with a circular corner radius of 4.0.
final ShapeBorder? shape; final ShapeBorder? shape;
/// The tile's internal padding. /// The tile's internal padding.
...@@ -1185,8 +1185,11 @@ class ListTile extends StatelessWidget { ...@@ -1185,8 +1185,11 @@ class ListTile extends StatelessWidget {
child: Semantics( child: Semantics(
selected: selected, selected: selected,
enabled: enabled, enabled: enabled,
child: ColoredBox( child: Ink(
color: _tileBackgroundColor(tileTheme), decoration: ShapeDecoration(
shape: shape ?? tileTheme.shape ?? const Border(),
color: _tileBackgroundColor(tileTheme),
),
child: SafeArea( child: SafeArea(
top: false, top: false,
bottom: false, bottom: false,
......
...@@ -577,7 +577,13 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver ...@@ -577,7 +577,13 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
@override @override
void applyPaintTransform(RenderBox child, Matrix4 transform) { 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 // It is possible that widgets under kept alive children want to paint
// themselves. For example, the Material widget tries to paint all // themselves. For example, the Material widget tries to paint all
// InkFeatures under its subtree as long as they are not disposed. In // InkFeatures under its subtree as long as they are not disposed. In
......
...@@ -129,7 +129,7 @@ void main() { ...@@ -129,7 +129,7 @@ void main() {
) )
); );
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 checkboxRect = tester.getRect(find.byType(Checkbox));
final Rect titleRect = tester.getRect(find.text('Title')); final Rect titleRect = tester.getRect(find.text('Title'));
...@@ -241,35 +241,34 @@ void main() { ...@@ -241,35 +241,34 @@ void main() {
}); });
testWidgets('CheckboxListTile respects tileColor', (WidgetTester tester) async { testWidgets('CheckboxListTile respects tileColor', (WidgetTester tester) async {
const Color tileColor = Colors.black; final Color tileColor = Colors.red.shade500;
await tester.pumpWidget( await tester.pumpWidget(
wrap( wrap(
child: const Center( child: Center(
child: CheckboxListTile( child: CheckboxListTile(
value: false, value: false,
onChanged: null, onChanged: null,
title: Text('Title'), title: const Text('Title'),
tileColor: tileColor, tileColor: tileColor,
), ),
), ),
), ),
); );
final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox)); expect(find.byType(Material), paints..path(color: tileColor));
expect(coloredBox.color, equals(tileColor));
}); });
testWidgets('CheckboxListTile respects selectedTileColor', (WidgetTester tester) async { testWidgets('CheckboxListTile respects selectedTileColor', (WidgetTester tester) async {
const Color selectedTileColor = Colors.black; final Color selectedTileColor = Colors.green.shade500;
await tester.pumpWidget( await tester.pumpWidget(
wrap( wrap(
child: const Center( child: Center(
child: CheckboxListTile( child: CheckboxListTile(
value: false, value: false,
onChanged: null, onChanged: null,
title: Text('Title'), title: const Text('Title'),
selected: true, selected: true,
selectedTileColor: selectedTileColor, selectedTileColor: selectedTileColor,
), ),
...@@ -277,7 +276,6 @@ void main() { ...@@ -277,7 +276,6 @@ void main() {
), ),
); );
final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox)); expect(find.byType(Material), paints..path(color: selectedTileColor));
expect(coloredBox.color, equals(selectedTileColor));
}); });
} }
...@@ -8,6 +8,7 @@ import 'package:flutter/rendering.dart'; ...@@ -8,6 +8,7 @@ import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import '../rendering/mock_canvas.dart';
import '../widgets/semantics_tester.dart'; import '../widgets/semantics_tester.dart';
...@@ -627,7 +628,7 @@ void main() { ...@@ -627,7 +628,7 @@ void main() {
) )
); );
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 radioRect = tester.getRect(find.byType(radioType));
final Rect titleRect = tester.getRect(find.text('Title')); final Rect titleRect = tester.getRect(find.text('Title'));
...@@ -667,37 +668,36 @@ void main() { ...@@ -667,37 +668,36 @@ void main() {
}); });
testWidgets('RadioListTile respects tileColor', (WidgetTester tester) async { testWidgets('RadioListTile respects tileColor', (WidgetTester tester) async {
const Color tileColor = Colors.red; final Color tileColor = Colors.red.shade500;
await tester.pumpWidget( await tester.pumpWidget(
wrap( wrap(
child: const Center( child: Center(
child: RadioListTile<bool>( child: RadioListTile<bool>(
value: false, value: false,
groupValue: true, groupValue: true,
onChanged: null, onChanged: null,
title: Text('Title'), title: const Text('Title'),
tileColor: tileColor, tileColor: tileColor,
), ),
), ),
), ),
); );
final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox)); expect(find.byType(Material), paints..path(color: tileColor));
expect(coloredBox.color, tileColor);
}); });
testWidgets('RadioListTile respects selectedTileColor', (WidgetTester tester) async { testWidgets('RadioListTile respects selectedTileColor', (WidgetTester tester) async {
const Color selectedTileColor = Colors.black; final Color selectedTileColor = Colors.green.shade500;
await tester.pumpWidget( await tester.pumpWidget(
wrap( wrap(
child: const Center( child: Center(
child: RadioListTile<bool>( child: RadioListTile<bool>(
value: false, value: false,
groupValue: true, groupValue: true,
onChanged: null, onChanged: null,
title: Text('Title'), title: const Text('Title'),
selected: true, selected: true,
selectedTileColor: selectedTileColor, selectedTileColor: selectedTileColor,
), ),
...@@ -705,7 +705,6 @@ void main() { ...@@ -705,7 +705,6 @@ void main() {
), ),
); );
final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox)); expect(find.byType(Material), paints..path(color: selectedTileColor));
expect(coloredBox.color, equals(selectedTileColor));
}); });
} }
...@@ -359,35 +359,34 @@ void main() { ...@@ -359,35 +359,34 @@ void main() {
}); });
testWidgets('SwitchListTile respects tileColor', (WidgetTester tester) async { testWidgets('SwitchListTile respects tileColor', (WidgetTester tester) async {
const Color tileColor = Colors.red; final Color tileColor = Colors.red.shade500;
await tester.pumpWidget( await tester.pumpWidget(
wrap( wrap(
child: const Center( child: Center(
child: SwitchListTile( child: SwitchListTile(
value: false, value: false,
onChanged: null, onChanged: null,
title: Text('Title'), title: const Text('Title'),
tileColor: tileColor, tileColor: tileColor,
), ),
), ),
), ),
); );
final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox)); expect(find.byType(Material), paints..path(color: tileColor));
expect(coloredBox.color, tileColor);
}); });
testWidgets('SwitchListTile respects selectedTileColor', (WidgetTester tester) async { testWidgets('SwitchListTile respects selectedTileColor', (WidgetTester tester) async {
const Color selectedTileColor = Colors.black; final Color selectedTileColor = Colors.green.shade500;
await tester.pumpWidget( await tester.pumpWidget(
wrap( wrap(
child: const Center( child: Center(
child: SwitchListTile( child: SwitchListTile(
value: false, value: false,
onChanged: null, onChanged: null,
title: Text('Title'), title: const Text('Title'),
selected: true, selected: true,
selectedTileColor: selectedTileColor, selectedTileColor: selectedTileColor,
), ),
...@@ -395,8 +394,7 @@ void main() { ...@@ -395,8 +394,7 @@ void main() {
), ),
); );
final ColoredBox coloredBox = tester.firstWidget(find.byType(ColoredBox)); expect(find.byType(Material), paints..path(color: selectedTileColor));
expect(coloredBox.color, equals(selectedTileColor));
}); });
} }
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
...@@ -21,14 +22,14 @@ class TestItem extends StatelessWidget { ...@@ -21,14 +22,14 @@ class TestItem extends StatelessWidget {
} }
} }
Widget buildFrame({ int? count, double? width, double? height, Axis? scrollDirection }) { Widget buildFrame({ int? count, double? width, double? height, Axis? scrollDirection, Key? prototypeKey }) {
return Directionality( return Directionality(
textDirection: TextDirection.ltr, textDirection: TextDirection.ltr,
child: CustomScrollView( child: CustomScrollView(
scrollDirection: scrollDirection ?? Axis.vertical, scrollDirection: scrollDirection ?? Axis.vertical,
slivers: <Widget>[ slivers: <Widget>[
SliverPrototypeExtentList( SliverPrototypeExtentList(
prototypeItem: TestItem(item: -1, width: width, height: height), prototypeItem: TestItem(item: -1, width: width, height: height, key: prototypeKey),
delegate: SliverChildBuilderDelegate( delegate: SliverChildBuilderDelegate(
(BuildContext context, int index) => TestItem(item: index), (BuildContext context, int index) => TestItem(item: index),
childCount: count, childCount: count,
...@@ -136,4 +137,18 @@ void main() { ...@@ -136,4 +137,18 @@ void main() {
for (int i = 1; i < 10; i += 1) for (int i = 1; i < 10; i += 1)
expect(find.text('Item $i'), findsOneWidget); 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());
});
} }
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