Skip to content
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

Add ripple effect style of new Android versions #49957

Open
sarsamurmu opened this issue Feb 2, 2020 · 16 comments
Open

Add ripple effect style of new Android versions #49957

sarsamurmu opened this issue Feb 2, 2020 · 16 comments
Labels
a: animation Animation APIs a: fidelity Matching the OEM platforms better a: quality A truly polished experience c: proposal A detailed proposal for a change to Flutter f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list team-design Owned by Design Languages team triaged-design Triaged by Design Languages team

Comments

@sarsamurmu
Copy link

Use case

Ripple effects of flutter app is looking kind of old fashioned in new Android versions.

Proposal

As you may know Android 9 introduced new animations, with those new animations we got a new kind of ripple effect animation, if there can be feature in flutter to use the new ripple effects that would look more native on new Android versions.

@dnfield dnfield added a: animation Animation APIs a: fidelity Matching the OEM platforms better a: quality A truly polished experience f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Feb 2, 2020
@dnfield dnfield added this to the Goals milestone Feb 2, 2020
@sarsamurmu
Copy link
Author

@dnfield After studying the Android 10 ripple effect animation carefully, I edited the code of ink_splash.dart which is available as gist here.

It's tested on InkWell with highlightColor: Colors.transparent.
Comparison of Android and Flutter ripple effect (YouTube video): Android and Flutter.

I hope it will help you at some point.

@justinmc
Copy link
Contributor

@sarsamurmu Would you be able to open a pull request with your changes? I'm happy to help with code review.

@sarsamurmu
Copy link
Author

@justinmc I'm kind of confused. Where should I open the PR? Making a new file? Where should I make the new file?

@justinmc
Copy link
Contributor

Fork the repository and clone the fork locally. Create a new branch. Make your changes to ink_splash.dart and commit. Push your commit to your branch. Then Github should prompt you to open a pull request if you visit the page for your fork in your browser.

Let me know if that doesn't make sense or if you need any help!

@sarsamurmu
Copy link
Author

@justinmc But what about that it needs highlightColor to be transparent?

@justinmc
Copy link
Contributor

I'm not sure, you might have to give me an example. Are you using something like withOpacity?

@sarsamurmu
Copy link
Author

See this - https://youtu.be/LdvmL9NRyOw. You can notice while the ripple is expanding, whole item is being highlighted with grey. In newer Android versions, there is no highlighting, only ripple. So if we want to implement the new ripple effect, we've to set highlightColor to transparent or it will not look like the original one. So, what we should do about the highlightColor?

@justinmc
Copy link
Contributor

Thanks for the video, I see what you mean about the difference now.

I'm not sure what to do about highlight color. Is it totally irrelevant now in the new Android 9 animation? It would be a breaking change, but maybe we could think about removing it...

@bernaferrari
Copy link
Contributor

bernaferrari commented Mar 27, 2020

@justinmc yes. You probably didn't realize, but if you get an Android device and tap anywhere, the effect is so fast and big that it totally replaces the highlightcolor.
I'm not sure how the highlightcolor interacts with Flutter Web, but it is worth looking if there will be any issues (like mouse hover)!

@sarsamurmu
Copy link
Author

Yes, the highlighting is totally irrelevant in newer Android versions.

@kf6gpe kf6gpe added the P2 Important issues not at the top of the work list label May 29, 2020
@TahaTesser TahaTesser added the c: proposal A detailed proposal for a change to Flutter label Jul 9, 2020
@Hixie Hixie removed this from the None. milestone Aug 17, 2020
@willlockwood
Copy link
Contributor

@bernaferrari @justinmc @sarsamurmu I'm interested in spending some time working to move a solution forward here, but I'm not completely clear on what would be the best path forward.

Would it be helpful if I opened a PR with @sarsamurmu's changes? Maybe an intermediate fix would exclude the removal of the highlight color, which could be addressed separately? Or I could open a PR with those changes, including the removal of the highlight color?

@willlockwood
Copy link
Contributor

willlockwood commented Dec 20, 2020

I've spent some time going through @sarsamurmu's gist, the current source code, and the commits that landed most of the code in the current source, and I'm not convinced that the gist above represents the most appropriate solution.

After coming to understand the current structure of the InteractiveInkFeature and its subclasses InkRipple/InkSplash, and knowing the direction that Google is going with the ripple (removing background highlight, making ripples take up more space in their containers), the question on my mind is whether the next iteration of how material ripples look in Flutter should be in the form of:

  1. Creating a new InteractiveInkFeature subclass that includes the new behavior (This currently makes the most sense to me, but I would have trouble with naming/conceptualizing—this would allow clients to use old and new ripple designs in the same app, for different android APIs)
  2. Modifying one of InkSplash/InkRipple to more closely resemble the newer material ripple for higher android API versions (this might include breaking changes, which we may be fine with, and could lead to inconsistent behavior between implementations that we'd want to be more consistent)
  3. Modifying both InkSplash and InkRipple to more closely resemble the newer material ripple for higher android API versions (this may also introduce breaking changes, and doesn't make much sense to me, because I think the implementations would roughly converge on having the same behavior?)
  4. Removing the abstraction, and moving forward with the ripple implementation that Google seems to be moving forward with, as a (I can't imagine this is the right choice, but it seems maybe to be the logical conclusion of the third option? But feels bad to me, to remove an abstraction put there to make moments like these easier).

It's hard for me to know which direction to head in, without having more clarity on:

  • exactly why we ended up with both InkSplash and InkRipple in the first place,
  • and whether the distinction between the two will likely survive the direction that the ripple is headed in,
  • more thoughts about how much material fidelity we'd like to aim for, in finding a good enough solution for the framework, and
  • whether the implementations of Chip or InkHighlight are relevant at all to this issue

So, specifically, I'm wondering if either @Hixie or @HansMuller can provide context here? Specifically, I'm trying to understand the original motivation behind #13986, which introduced the abstraction that I'm now questioning.

Looking forward to being corrected/getting more context on any of this—thank y'all!

cc @bernaferrari @justinmc

@Hixie
Copy link
Contributor

Hixie commented Dec 20, 2020

The ripple effect is configurable in the Theme, right?

@willlockwood
Copy link
Contributor

So yes, you can choose which factory you use for the ripple—I think the issue here is that both the InkRipple and InkSplash seem to no longer match up with the newer android material ripple designs.

So depending on the original purpose is of having each of these factories (is InkRipple supposed to be an updated version of InkSplash? Or an InkSplash without the now-irrelevant background highlight color? should these now represent outdated material designs that we want to keep available?), how to update them to more closely resemble the latest android material ripples may vary a bit. Would you happen to have any more insight about that distinction?

@Hixie
Copy link
Contributor

Hixie commented Dec 20, 2020

If it's configurable, I'd probably recommend making a new class that does the new style, and having the default theme use that class (with documentation showing how to use the other classes, not to mention making your own, etc). Presumably the highlight color is also configurable, so same deal there.

@HansMuller would know if the various classes are supposed to match the new style or if they're different older variants and we need yet another third type of ripple.

@willlockwood
Copy link
Contributor

Great—thanks for the quick response/input! I can get started on an implementation and respond to any incoming input/context from @HansMuller 👍

@flutter-triage-bot flutter-triage-bot bot added team-design Owned by Design Languages team triaged-design Triaged by Design Languages team labels Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: animation Animation APIs a: fidelity Matching the OEM platforms better a: quality A truly polished experience c: proposal A detailed proposal for a change to Flutter f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list team-design Owned by Design Languages team triaged-design Triaged by Design Languages team
Projects
None yet
Development

No branches or pull requests

8 participants