Fix InkWell overlayColor resolution ignores selected state #159072

bleroux · 2024-11-18T13:19:13Z

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:

flutter/packages/flutter/lib/src/material/date_picker_theme.dart

Lines 982 to 1006 in dc44547

    
           MaterialStateProperty<Color?>? get dayOverlayColor => 
        
             MaterialStateProperty.resolveWith((Set<MaterialState> states) { 
        
               if (states.contains(MaterialState.selected)) { 
        
                 if (states.contains(MaterialState.pressed)) { 
        
                   return _colors.onPrimary.withOpacity(0.1); 
        
                 } 
        
                 if (states.contains(MaterialState.hovered)) { 
        
                   return _colors.onPrimary.withOpacity(0.08); 
        
                 } 
        
                 if (states.contains(MaterialState.focused)) { 
        
                   return _colors.onPrimary.withOpacity(0.1); 
        
                 } 
        
               } else { 
        
                 if (states.contains(MaterialState.pressed)) { 
        
                   return _colors.onSurfaceVariant.withOpacity(0.1); 
        
                 } 
        
                 if (states.contains(MaterialState.hovered)) { 
        
                   return _colors.onSurfaceVariant.withOpacity(0.08); 
        
                 } 
        
                 if (states.contains(MaterialState.focused)) { 
        
                   return _colors.onSurfaceVariant.withOpacity(0.1); 
        
                 } 
        
               } 
        
               return null; 
        
             });

Related Issue

Fixes InkWell overlay colors aren't applied on MaterialState.selected state
First step for Date picker overlay colors aren't applied on MaterialState.selected state.

Tests

Adds 3 tests.

justinmc

LGTM with a few typo nits in the docs 👍

justinmc · 2024-11-18T21:31:20Z

packages/flutter/lib/src/material/ink_well.dart

+    const Set<MaterialState> highlightableStates = <MaterialState>{MaterialState.focused, MaterialState.hovered, MaterialState.pressed};
+    final Set<MaterialState> nonHighlightableStates = statesController.value.difference(highlightableStates);
+    // Each highlightable states will be resolved separatly to get the corresponding color.
+    // For this resolution to be correct, The non-highlightable states should be preserved.


"The" => "the"

justinmc · 2024-11-18T21:35:09Z

packages/flutter/lib/src/material/ink_well.dart

+    final ThemeData theme = Theme.of(context);
+    const Set<MaterialState> highlightableStates = <MaterialState>{MaterialState.focused, MaterialState.hovered, MaterialState.pressed};
+    final Set<MaterialState> nonHighlightableStates = statesController.value.difference(highlightableStates);
+    // Each highlightable states will be resolved separatly to get the corresponding color.


"states" => "state"
"separatly" => "separately"

justinmc · 2024-11-18T21:41:55Z

packages/flutter/test/material/ink_well_test.dart

 import '../widgets/semantics_tester.dart';

 void main() {
+  RenderObject getInkFeatures(WidgetTester tester) {


Good call splitting this out.

justinmc · 2024-11-18T21:46:15Z

packages/flutter/lib/src/material/ink_well.dart

+    final Set<MaterialState> nonHighlightableStates = statesController.value.difference(highlightableStates);
+    // Each highlightable states will be resolved separatly to get the corresponding color.
+    // For this resolution to be correct, The non-highlightable states should be preserved.
+    final Set<MaterialState> pressed = <MaterialState>{...nonHighlightableStates, MaterialState.pressed};


This is kind of tricky, I hope we don't make this same kind of mistake elsewhere. I guess it's probably rare to speculatively resolve a color like this, though.

I hope we don't make this same kind of mistake elsewher

I looked at various places where the color are resolved. I think this one is somewhat particular because most of InkWell logic was written way before widget state controllers were introduced. For instance the getHighlightColorForType function was added in #31438 and then was updated and moved later.
This logic can probably be simplified, but because this is a core widget I tried to fix the issue without taking risk.

bleroux · 2024-11-18T22:29:40Z

@justinmc Thanks for the review, especially for catching these typos. 🙏

Let see how it goes with Google testing because this might break some golden (if there are golden capturing hovering, focused or pressed state). For instance, with this fix hovered color for some widgets which have a selected state (such as IconButton) is slightly different (the change is slight because the defaults colors are very close but this could be noticed when comparing screenshots.
For instance, here are screenshots of an icon button created from IconButton.filledTonal constructor when hovered:

Before	After

…lutter/flutter#159072)

flutter/flutter@b3818f6...8536b96 2024-11-19 [email protected] Terminate `flutter test` when no longer needed in integration test. (flutter/flutter#159117) 2024-11-19 [email protected] Terminate the test device if the `flutter` tool is signal-killed. (flutter/flutter#159115) 2024-11-19 [email protected] Roll Packages from c1eabf5 to fc4adc7 (10 revisions) (flutter/flutter#159143) 2024-11-19 [email protected] Deflake api 35 emulator tests by updating emulator definitions version to latest available from chrome infra (flutter/flutter#158017) 2024-11-19 [email protected] Roll Flutter Engine from b6723e33b858 to cff1e751f853 (1 revision) (flutter/flutter#159141) 2024-11-19 [email protected] Fix InkWell overlayColor resolution ignores selected state (flutter/flutter#159072) 2024-11-19 [email protected] Roll Flutter Engine from 983b7b85d122 to b6723e33b858 (3 revisions) (flutter/flutter#159134) 2024-11-19 [email protected] Roll Flutter Engine from c1b0e18a70b3 to 983b7b85d122 (2 revisions) (flutter/flutter#159124) 2024-11-19 [email protected] Roll Flutter Engine from 878f593802e1 to c1b0e18a70b3 (13 revisions) (flutter/flutter#159118) 2024-11-19 [email protected] [SwiftPM] Move where the migration checks feature flags (flutter/flutter#159110) 2024-11-18 [email protected] Plumbs `scrollBehavior` into `SelectableText` so that the scrollbar may be hidden (flutter/flutter#158887) 2024-11-18 [email protected] Add a tag and assert some state in FlutterTestDriver tests. (flutter/flutter#159099) 2024-11-18 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.6.0 to 5.0.2 in the all-github-actions group (flutter/flutter#159104) 2024-11-18 [email protected] Add `@protected` to public `State` method overrides (flutter/flutter#157313) 2024-11-18 [email protected] Roll Flutter Engine from 9686de154114 to 878f593802e1 (5 revisions) (flutter/flutter#159097) 2024-11-18 [email protected] Fix flaky failure related to core_device_list.json not being found (flutter/flutter#158946) 2024-11-18 [email protected] Prettier merge_queue.md (flutter/flutter#158969) 2024-11-18 [email protected] Define and use `flutterBin` consistently across `integration.shard`. (flutter/flutter#159007) 2024-11-18 [email protected] No longer download `android-x86-jit-release`. (flutter/flutter#159011) 2024-11-18 [email protected] Roll Flutter Engine from e1f4e7d9bfc4 to 9686de154114 (1 revision) (flutter/flutter#159082) 2024-11-18 [email protected] Roll Flutter Engine from f365c9f5dce3 to e1f4e7d9bfc4 (4 revisions) (flutter/flutter#159078) 2024-11-18 [email protected] Roll Packages from b164be3 to c1eabf5 (6 revisions) (flutter/flutter#159077) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

) ## Description This PR fixes the DatePicker overlay colors for the selected days. Before this PR, overlays were obscured by the selected day backgound. This fix simply replaces a DecoratedBox with an Ink to make the overlays visible. Combined with #159072 which fixes InkWell overlay color resolution related to the selected state, this PR fixes [Date picker overlay colors aren't applied on MaterialState.selected state](#130586). Before, no overlay visible for the selected day when hovered, focused, or pressed: https://github.com/user-attachments/assets/944d5035-68b2-40da-b606-3e8795229767 After, overlay is visible for the selected day when hovered, focused, or pressed (color change is slight as defined with M3 defaults): https://github.com/user-attachments/assets/2627955b-f45a-465f-8eb0-21955ccd8c3e ## Related Issue Fixes [Date picker overlay colors aren't applied on MaterialState.selected state](#130586). ## Tests Adds 12 tests. Updates several existing tests (those tests were looking for a `DecoratedBox`, make them look for an `Ink`).

#159583) Reverts #159203 because it depends on #159072 which was flagged as a perf regression in #159337. Reverting both PRs to see if the perf regression was really related to this change or was impacted by another change. See #159337 (comment) for context.

…lutter#159072)" This reverts commit 043c59b.

…159072) (#159589) Reverts #159072 which was flagged as a perf regression in #159337. Reverting to see if the perf regression was really related to this change or was impacted by another change. See #159337 (comment) for context.

…59784) Reland #159072 without change. The initial PR was flagged for a non-related perf regression, see #159337 (comment) Fixes #159063

…lutter/flutter#159072)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Nov 18, 2024

bleroux force-pushed the fix_ink_well_overlay_color_resolution_ignores_selected_state branch from 6c2fe5e to 0612cec Compare November 18, 2024 15:11

bleroux requested a review from justinmc November 18, 2024 15:38

justinmc approved these changes Nov 18, 2024

View reviewed changes

bleroux added 2 commits November 19, 2024 09:17

Fix InkWell overlayColor resolution ignores selected state

6aaf6c6

Fix typos in comment

9ff6c2f

bleroux force-pushed the fix_ink_well_overlay_color_resolution_ignores_selected_state branch from 0612cec to 9ff6c2f Compare November 19, 2024 08:22

bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 19, 2024

auto-submit bot added this pull request to the merge queue Nov 19, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 19, 2024

Merged via the queue into flutter:master with commit 043c59b Nov 19, 2024
79 checks passed

engine-flutter-autoroll mentioned this pull request Nov 19, 2024

Roll Flutter from b3818f6b5979 to 043c59b07525 (17 revisions) flutter/packages#8121

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 19, 2024

043c59b Fix InkWell overlayColor resolution ignores selected state (f…

a211cc3

…lutter/flutter#159072)

engine-flutter-autoroll mentioned this pull request Nov 19, 2024

Roll Flutter from b3818f6b5979 to 87872a609538 (18 revisions) flutter/packages#8122

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 19, 2024

043c59b Fix InkWell overlayColor resolution ignores selected state (f…

d45c79d

…lutter/flutter#159072)

engine-flutter-autoroll mentioned this pull request Nov 19, 2024

Roll Flutter from b3818f6b5979 to 0e1c6330ea7d (20 revisions) flutter/packages#8123

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 19, 2024

043c59b Fix InkWell overlayColor resolution ignores selected state (f…

2dba8d2

…lutter/flutter#159072)

engine-flutter-autoroll mentioned this pull request Nov 19, 2024

Roll Flutter from b3818f6b5979 to 8536b96ebb3e (22 revisions) flutter/packages#8124

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 19, 2024

043c59b Fix InkWell overlayColor resolution ignores selected state (f…

97e2e3c

…lutter/flutter#159072)

bleroux deleted the fix_ink_well_overlay_color_resolution_ignores_selected_state branch November 19, 2024 18:38

bleroux mentioned this pull request Nov 20, 2024

Fix Date picker overlay colors aren't applied on selected state #159203

Merged

gaaclarke mentioned this pull request Nov 22, 2024

2x regression in total_ui_gc_time for backdrop_filter_perf_ios__timeline_summary #159337

Closed

This was referenced Nov 27, 2024

Optimize InkWell widget states resolution #159546

Closed

Revert "Fix Date picker overlay colors aren't applied on selected sta… #159583

Merged

bleroux added a commit to NevercodeHQ/flutter that referenced this pull request Nov 28, 2024

Revert "Fix InkWell overlayColor resolution ignores selected state (f…

656591f

…lutter#159072)" This reverts commit 043c59b.

bleroux mentioned this pull request Nov 28, 2024

Revert "Fix InkWell overlayColor resolution ignores selected state (#159072) #159589

Merged

This was referenced Nov 29, 2024

InkWell overlay colors aren't applied on MaterialState.selected state #159063

Closed

Reland Fix InkWell overlayColor resolution ignores selected state #159784

Merged

reidbaker mentioned this pull request Dec 13, 2024

Cutout rotation integration test #160255

Closed

11 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

043c59b Fix InkWell overlayColor resolution ignores selected state (f…

ff0403f

…lutter/flutter#159072)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

043c59b Fix InkWell overlayColor resolution ignores selected state (f…

06f58f7

…lutter/flutter#159072)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

043c59b Fix InkWell overlayColor resolution ignores selected state (f…

9a998c5

…lutter/flutter#159072)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

043c59b Fix InkWell overlayColor resolution ignores selected state (f…

1a0671d

…lutter/flutter#159072)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025

043c59b Fix InkWell overlayColor resolution ignores selected state (f…

999018d

…lutter/flutter#159072)

bleroux mentioned this pull request Jul 28, 2025

[Material3] SegmentedButton sets its MaterialState incorrectly for overlayColor #123308

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix InkWell overlayColor resolution ignores selected state #159072

Fix InkWell overlayColor resolution ignores selected state #159072

Uh oh!

bleroux commented Nov 18, 2024 •

edited

Loading

Uh oh!

justinmc left a comment

Uh oh!

justinmc Nov 18, 2024

Uh oh!

justinmc Nov 18, 2024

Uh oh!

justinmc Nov 18, 2024

Uh oh!

justinmc Nov 18, 2024

Uh oh!

bleroux Nov 18, 2024

Uh oh!

bleroux commented Nov 18, 2024 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	MaterialStateProperty<Color?>? get dayOverlayColor =>
	MaterialStateProperty.resolveWith((Set<MaterialState> states) {
	if (states.contains(MaterialState.selected)) {
	if (states.contains(MaterialState.pressed)) {
	return _colors.onPrimary.withOpacity(0.1);
	}
	if (states.contains(MaterialState.hovered)) {
	return _colors.onPrimary.withOpacity(0.08);
	}
	if (states.contains(MaterialState.focused)) {
	return _colors.onPrimary.withOpacity(0.1);
	}
	} else {
	if (states.contains(MaterialState.pressed)) {
	return _colors.onSurfaceVariant.withOpacity(0.1);
	}
	if (states.contains(MaterialState.hovered)) {
	return _colors.onSurfaceVariant.withOpacity(0.08);
	}
	if (states.contains(MaterialState.focused)) {
	return _colors.onSurfaceVariant.withOpacity(0.1);
	}
	}
	return null;
	});

Fix InkWell overlayColor resolution ignores selected state #159072

Fix InkWell overlayColor resolution ignores selected state #159072

Uh oh!

Conversation

bleroux commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Tests

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

bleroux Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

bleroux commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bleroux commented Nov 18, 2024 •

edited

Loading

bleroux commented Nov 18, 2024 •

edited

Loading