From 043c59b07525e849e6d12200c9ac97bf74a9b53f Mon Sep 17 00:00:00 2001 From: Bruno Leroux Date: Tue, 19 Nov 2024 12:33:40 +0100 Subject: [PATCH] Fix InkWell overlayColor resolution ignores selected state (#159072) ## Description This PR fixes `InkWell` overlay colors resolution. The `InkWell` overlay color is resolved when the `InkWell` is either focused, hovered , and/or pressed. This resolution happens at two places: - first when the highlight is created. - then on each build, using the inner function named `getHighlightColorForType`. This second resolution should be aware of other current states (such as selected) as it might impact the color resolution. For instance, several Material styles have colors resolution define similarly to: https://github.com/flutter/flutter/blob/dc44547d0d90dfff4fd4c3bdcd525f9d7a70744e/packages/flutter/lib/src/material/date_picker_theme.dart#L982-L1006 ## Related Issue Fixes [InkWell overlay colors aren't applied on MaterialState.selected state](https://github.com/flutter/flutter/issues/159063) First step for [Date picker overlay colors aren't applied on MaterialState.selected state](Date picker overlay colors aren't applied on MaterialState.selected state). ## Tests Adds 3 tests. --- .../flutter/lib/src/material/ink_well.dart | 14 +- .../flutter/test/material/ink_well_test.dart | 136 +++++++++++++++--- 2 files changed, 123 insertions(+), 27 deletions(-) diff --git a/packages/flutter/lib/src/material/ink_well.dart b/packages/flutter/lib/src/material/ink_well.dart index 4be927cf18..6a0e37cab5 100644 --- a/packages/flutter/lib/src/material/ink_well.dart +++ b/packages/flutter/lib/src/material/ink_well.dart @@ -1283,12 +1283,16 @@ class _InkResponseState extends State<_InkResponseStateWidget> assert(widget.debugCheckContext(context)); super.build(context); // See AutomaticKeepAliveClientMixin. - Color getHighlightColorForType(_HighlightType type) { - const Set pressed = {MaterialState.pressed}; - const Set focused = {MaterialState.focused}; - const Set hovered = {MaterialState.hovered}; + final ThemeData theme = Theme.of(context); + const Set highlightableStates = {MaterialState.focused, MaterialState.hovered, MaterialState.pressed}; + final Set nonHighlightableStates = statesController.value.difference(highlightableStates); + // Each highlightable state will be resolved separately to get the corresponding color. + // For this resolution to be correct, the non-highlightable states should be preserved. + final Set pressed = {...nonHighlightableStates, MaterialState.pressed}; + final Set focused = {...nonHighlightableStates, MaterialState.focused}; + final Set hovered = {...nonHighlightableStates, MaterialState.hovered}; - final ThemeData theme = Theme.of(context); + Color getHighlightColorForType(_HighlightType type) { return switch (type) { // The pressed state triggers a ripple (ink splash), per the current // Material Design spec. A separate highlight is no longer used. diff --git a/packages/flutter/test/material/ink_well_test.dart b/packages/flutter/test/material/ink_well_test.dart index 3cc961ccc1..9de987c354 100644 --- a/packages/flutter/test/material/ink_well_test.dart +++ b/packages/flutter/test/material/ink_well_test.dart @@ -11,6 +11,10 @@ import '../widgets/feedback_tester.dart'; import '../widgets/semantics_tester.dart'; void main() { + RenderObject getInkFeatures(WidgetTester tester) { + return tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + } + testWidgets('InkWell gestures control test', (WidgetTester tester) async { final List log = []; @@ -170,7 +174,7 @@ void main() { await gesture.addPointer(); await gesture.moveTo(tester.getCenter(find.byType(SizedBox))); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paints..rect(rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0), color: const Color(0xff00ff00))); }); @@ -209,7 +213,7 @@ void main() { await gesture.addPointer(); await gesture.moveTo(tester.getCenter(find.byType(SizedBox))); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paints..rect(rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0), color: const Color(0xff00ff00))); }); @@ -240,7 +244,7 @@ void main() { ), ); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawRect, 0)); focusNode.requestFocus(); await tester.pumpAndSettle(); @@ -289,7 +293,7 @@ void main() { ), ); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawRect, 0)); focusNode.requestFocus(); await tester.pumpAndSettle(); @@ -327,13 +331,101 @@ void main() { )); await tester.pumpAndSettle(); final TestGesture gesture = await tester.startGesture(tester.getRect(find.byType(InkWell)).center); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paints..rect(rect: const Rect.fromLTRB(0, 0, 100, 100), color: pressedColor.withAlpha(0))); await tester.pumpAndSettle(); // Let the press highlight animation finish. expect(inkFeatures, paints..rect(rect: const Rect.fromLTRB(0, 0, 100, 100), color: pressedColor)); await gesture.up(); }); + group('Ink well overlayColor resolution respects WidgetState.selected', () { + const Color selectedHoveredColor = Color(0xff00ff00); + const Color selectedFocusedColor = Color(0xff0000ff); + const Color selectedPressedColor = Color(0xff00ffff); + const Rect inkRect = Rect.fromLTRB(0, 0, 100, 100); + + Widget boilerplate({ FocusNode? focusNode }) { + final WidgetStatesController statesController = WidgetStatesController({MaterialState.selected}); + addTearDown(statesController.dispose); + + return Material( + child: Directionality( + textDirection: TextDirection.ltr, + child: Align( + alignment: Alignment.topLeft, + child: SizedBox( + width: 100, + height: 100, + child: InkWell( + splashFactory: NoSplash.splashFactory, + focusNode: focusNode, + statesController: statesController, + overlayColor: WidgetStateProperty.resolveWith((Set states) { + if (states.contains(WidgetState.selected)) { + if (states.contains(WidgetState.pressed)) { + return selectedPressedColor; + } + if (states.contains(WidgetState.hovered)) { + return selectedHoveredColor; + } + if (states.contains(WidgetState.focused)) { + return selectedFocusedColor; + } + return const Color(0xffbadbad); // Shouldn't happen. + } else { + return Colors.black; + } + }), + onTap: () { }, + ), + ), + ), + ), + ); + } + + testWidgets('when focused', (WidgetTester tester) async { + FocusManager.instance.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; + final FocusNode focusNode = FocusNode(debugLabel: 'Ink Focus'); + addTearDown(focusNode.dispose); + + await tester.pumpWidget(boilerplate(focusNode: focusNode)); + await tester.pumpAndSettle(); + + final RenderObject inkFeatures = getInkFeatures(tester); + expect(inkFeatures, paintsExactlyCountTimes(#drawRect, 0)); + focusNode.requestFocus(); + await tester.pumpAndSettle(); + + expect(inkFeatures, paints..rect(rect: inkRect, color: selectedFocusedColor)); + }); + + testWidgets('when hovered', (WidgetTester tester) async { + await tester.pumpWidget(boilerplate()); + await tester.pumpAndSettle(); + + final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse); + await gesture.addPointer(); + await gesture.moveTo(tester.getCenter(find.byType(SizedBox))); + await tester.pumpAndSettle(); + + final RenderObject inkFeatures = getInkFeatures(tester); + expect(inkFeatures, paints..rect(rect: inkRect, color: selectedHoveredColor)); + }); + + testWidgets('when pressed', (WidgetTester tester) async { + await tester.pumpWidget(boilerplate()); + await tester.pumpAndSettle(); + + final TestGesture gesture = await tester.startGesture(tester.getRect(find.byType(InkWell)).center); + final RenderObject inkFeatures = getInkFeatures(tester); + expect(inkFeatures, paints..rect(rect: inkRect, color: selectedPressedColor.withAlpha(0))); + await tester.pumpAndSettle(); // Let the press highlight animation finish. + expect(inkFeatures, paints..rect(rect: inkRect, color: selectedPressedColor)); + await gesture.up(); + }); + }); + testWidgets('ink response splashColor matches splashColor parameter', (WidgetTester tester) async { FocusManager.instance.highlightStrategy = FocusHighlightStrategy.alwaysTouch; final FocusNode focusNode = FocusNode(debugLabel: 'Ink Focus'); @@ -367,7 +459,7 @@ void main() { await tester.pumpAndSettle(); final TestGesture gesture = await tester.startGesture(tester.getRect(find.byType(InkWell)).center); await tester.pump(const Duration(milliseconds: 200)); // unconfirmed splash is well underway - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paints..circle(x: 50, y: 50, color: splashColor)); await gesture.up(); focusNode.dispose(); @@ -417,7 +509,7 @@ void main() { await tester.pumpAndSettle(); final TestGesture gesture = await tester.startGesture(tester.getRect(find.byType(InkWell)).center); await tester.pump(const Duration(milliseconds: 200)); // unconfirmed splash is well underway - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paints..circle(x: 50, y: 50, color: splashColor)); await gesture.up(); focusNode.dispose(); @@ -446,7 +538,7 @@ void main() { ), ); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawCircle, 0)); focusNode.requestFocus(); await tester.pumpAndSettle(); @@ -477,7 +569,7 @@ void main() { ), ); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawRRect, 0)); focusNode.requestFocus(); @@ -513,7 +605,7 @@ void main() { ), ); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawRRect, 0)); // Hover the ink well. @@ -555,7 +647,7 @@ void main() { ), ); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#clipPath, 0)); expect(inkFeatures, paintsExactlyCountTimes(#drawRRect, 0)); @@ -607,7 +699,7 @@ void main() { ), ); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#clipPath, 0)); expect(inkFeatures, paintsExactlyCountTimes(#drawRRect, 0)); @@ -660,7 +752,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { } await tester.pumpWidget(boilerplate(10)); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawCircle, 0)); focusNode.requestFocus(); @@ -701,7 +793,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { await tester.pumpWidget(boilerplate(BoxShape.circle)); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawCircle, 0)); expect(inkFeatures, paintsExactlyCountTimes(#drawRRect, 0)); @@ -742,7 +834,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { await tester.pumpWidget(boilerplate(BorderRadius.circular(10))); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawRRect, 0)); focusNode.requestFocus(); @@ -791,7 +883,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { await tester.pumpWidget(boilerplate(BorderRadius.circular(20))); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#clipPath, 0)); focusNode.requestFocus(); @@ -862,7 +954,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { await tester.pumpWidget(boilerplate(BorderRadius.circular(20))); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#clipPath, 0)); final TestGesture gesture = await tester.startGesture(tester.getRect(find.byType(InkWell)).center); @@ -945,7 +1037,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { ), )); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawRect, 0)); focusNode.requestFocus(); await tester.pumpAndSettle(); @@ -2048,7 +2140,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { expect(log, equals(['tap-down', 'double-tap'])); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawRect, 0)); }); @@ -2093,7 +2185,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { expect(log, equals(['tap-down', 'tap-down', 'tap-cancel', 'double-tap'])); await tester.pumpAndSettle(); - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawCircle, 0)); }); @@ -2171,7 +2263,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { await tester.pumpAndSettle(); await gesture.moveTo(const Offset(10, 10)); // fade out the overlay await tester.pump(); // trigger the fade out animation - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); // Fadeout begins with the MaterialStates.hovered overlay color expect(inkFeatures, paints..rect(rect: const Rect.fromLTRB(350.0, 250.0, 450.0, 350.0), color: const Color(0xff00ff00))); // 50ms fadeout is 50% complete, overlay color alpha goes from 0xff to 0x80 @@ -2236,7 +2328,7 @@ testWidgets('InkResponse radius can be updated', (WidgetTester tester) async { await tester.pump(const Duration(milliseconds: 200)); // No splash should be painted. - final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) => object.runtimeType.toString() == '_RenderInkFeatures'); + final RenderObject inkFeatures = getInkFeatures(tester); expect(inkFeatures, paintsExactlyCountTimes(#drawCircle, 0)); await gesture.up();