-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
Note: Design doc/proposal is in progress; will link it here when ready 👍🏻
Pre-issue notes:
This issue is intended to organize/contextualize the work for several small PRs, related to various possible improvements to the user's and developer's interactions with the InkRipple and InkSplash. For all of the suggestions written below, unless explicitly specified otherwise, I've already gone through the process of making the improvements on other branches, but have decided to split up the work to make review/merging easier for everyone.
So, this is intended to be an issue spanning many small improvements, which I plan to address in individual PRs. Many of the small improvements have been brought up in other issues; the work described below fully encapsulates the improvements described in at least issues #20922 and #49957).
This issue also presupposes the virtue of the point made by Marc Glasberg here: #20922 (comment)
But users couldn't care less about the Material guidelines. They want a consistent experience, and they will judge your app according to that.... I'd suggest Flutter follows Android and you may open a ticket on the Material guidelines instead.
I work in apps that are being migrated from Android to Flutter, and can attest to how annoying it is to have different implementations of the same view in one user experience. If anyone disagrees vehemently with this general point, I'd be happy to chat about it and come to a consensus on how to proceed.
Purpose:
This issue generally addresses the many small ways that the implementation and documentation of ink features (particularly, the InkSplash and InkRipple) can be improved. The purposes of these two implementations should be more clear for developers, and the implementations themselves should more accurately reflect the way they're implemented on Android, where users may see Flutter material widgets in the same user experience as an Android material view.
Proposed/Planned Changes:
I've broken down the improvements into buckets of work that can be addressed in individual, smaller PRs. I have code written already, and I'm planning on putting up PRs for each of these, in the order that they're written here. Click below each for screenshots demonstrating the inconsistencies between Android and Flutter.
-
SplashFactory Test Coverage: Currently, many tests fail after changing the default splash factory on the material Theme. To make the PR changing the default splash factory easier, test coverage should be improved for widgets using ink features, to be able to pass regardless of what the theme’s default splash factory is. We can take the opportunity here to consolidate some duplicated code for utils related to testing how ripples are drawn.
-
Default SplashFactory: As a quick follow-up to the first PR, we should also change the default splash factory to the
InkRipple.splashFactory, which is going to match the material animation for all Android APIs going forward. Currently,InkSplash.splashFactoryis the default, which is an outdated choice, that's getting more outdated by the day. -
SplashFactory Documentation:
InkRipple/InkSplashdocumentation should be more clear about their intended use cases. Specifically, it should state that theInkRippleis an updated version of the InkSplash, and that the two implementations represent the ripple animations seen on Android versions 9 and above (API 28+), and Android versions 5-8 (API 21-27), respectively.
-
Ripple Color: The Flutter material ripple color is not exactly correct, which causes a slight inconsistency, most clearly visible in the colors of overlapping ripples, or on ripples on non-white backgrounds.
-
Ripple Initial Radius: The initial radius of the
InkRippleis 30% of its container’s largest dimension, but it should be 60%. Documentation of the Flutter implementation shows that it was intended to be 60%, so this is just a bug in the original implementation.
-
Ripple Target Radius: The target radius of the
InkRipple/InkSplashnot only lacks fidelity to the Android version (which is simpler/more readable, and more sensible), but is unnecessarily non-performant. Use the simpler logic, used in the Android source.
Screenshots:
InkSplash:
The difference for theInkRippleis really hard to spot, but I'll make it clear in my PR. basically, you can't actually tell the difference of the size, except in how the target size affects the timing of the ripple's fade out animation. Especially with the other issues, it's not really worth adding a screenshot here for now. -
Position/Curves for Ripples/Splashes: The initial position/position of ink features, as well as the curves they follow as they expand, lacks fidelity to the Android implementation. The difference in initial position is more noticeable, but these are likely good things to address at the same time.
-
Removing
InkRipple's highlight TheInkRippleshould not have associatedInkHighlightsdrawn for pressed gesture states. Remove it (or just make it fully transparent).
-
Ink Feature Cancel Tap Logic: The tap logic for when to initiate a splash is incorrect, as pointed out also by Marc Glasberg here: InkWell and InkResponse not close enough to the real thing #20922 (comment). It should be initiated on cancel or confirm of a tap, instead of on tapDown.
- Currently, it isn’t possible to make this completely in line with the Android implementation, because cancel events happen much more often in Flutter (cancels happen automatically after dragging far enough away from a pointer’s origin), but we can at least still trigger splashes on cancel, regardless of what triggers those cancellations.
- (Probably a separate issue/needs input) So, separately, it would be nice to also address the limitation that we seem to have set on gestures for cancellation, where a tap is canceled after any drag event brings the pointer a 20 logical pixels away from its origin. On android, we are able to drag anywhere in the bounds of the gesture detector, without triggering a cancellation. I think this is clearly a preferred outcome, but I’m unsure about why it was done like this in the first place, and am curious what else I haven’t considered that might have made this limitation necessary.
TL;DR:
Basically, there are a bunch of ways we can improve the Ripple/Splash animations, to have higher quality and fidelity to the Android versions. I have a bunch of the work planned and written, and I'm planning on opening PRs that just go down the list above. At the end of all this, the InkSplash an InkRipple should look pretty damn close to the way they do on Android, and hopefully, implementation/review will be pretty smooth/painless.















