-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Fix Date picker overlay colors aren't applied on selected state #159203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Date picker overlay colors aren't applied on selected state #159203
Conversation
1f12bf0 to
461ca17
Compare
461ca17 to
872dfda
Compare
| MaterialInkController findDayGridMaterial(WidgetTester tester) { | ||
| // All days are painted on the same Material widget. | ||
| // Use an arbitrary day to find this Material. | ||
| return Material.of(tester.element(find.text('17'))); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍😍
This is wonderful, thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
It took me several hours to figure out that there is not one Material for each day.
This was mainly because existing tests were carefully using the day they are interested in to look up the Material. It was ok, until I spent hours fixing one test where the circle painted for another day collides with the one I was interested in.
I created this utility function to save time for future contributors who will work on the date picker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some mixed feelings about the Ink widget—it seems like it'd help performance by consolidating a bunch of widgets into a single ink controller, but it also has the tendency to introduce jank, and while it appears to work very similar to DecoratedBox, some things are super confusing for anyone who doesn't have a solid grasp of how painting on a Material works.
TahaTesser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Reason for revert: This PR depends on #159072 which was flagged as a perf regression in #159337. Reverting to see if the perf regression was real or not. See #159337 (comment) for context. |
|
Time to revert pull request flutter/flutter/159203 has elapsed. |
…te (flutter#159203)" This reverts commit 8509d78.
#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.
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.
Before, no overlay visible for the selected day when hovered, focused, or pressed:
Enregistrement.de.l.ecran.2024-11-20.a.16.10.38.mov
After, overlay is visible for the selected day when hovered, focused, or pressed (color change is slight as defined with M3 defaults):
Enregistrement.de.l.ecran.2024-11-20.a.16.09.14.mov
Related Issue
Fixes Date picker overlay colors aren't applied on MaterialState.selected state.
Tests
Adds 12 tests.
Updates several existing tests (those tests were looking for a
DecoratedBox, make them look for anInk).