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

Workaround: add button on top to handle ripple #27

Closed
wants to merge 1 commit into from

Conversation

bounty1342
Copy link

https://stackoverflow.com/questions/48066321/inkwell-ripple-over-top-of-image-in-gridtile-in-flutter

Maybe some reffactor needed, or expose the color of the ripple effect can be usefull.

Hope this help.

Ripple on Google

@bounty1342 bounty1342 changed the title Workaround: add button on top to hanble ripple Workaround: add button on top to handle ripple Nov 26, 2019
@ZaynJarvis
Copy link
Owner

LGTM; however, width and margin looks like hard code number to me. Will check before merge in next release

@ZaynJarvis ZaynJarvis mentioned this pull request Feb 22, 2020
@kkizlaitis
Copy link

I checked out this PR and although at first glance it seemed fine, I noticed a bug with it.

In @bounty1342 's implementation, now multiple widgets get the onPressed variable: InkWell and SignInButtonBuilder (which is then passed to MaterialButton). The splash effect indeed works fine when pressing on the middle of the button. The problem is that MaterialButton is bigger than InkWell, so you can press a bit outside of the button and the touch will be passed to MaterialButton but not to InkWell. So the splash bug still remains in that scenario :/

@bounty1342
Copy link
Author

I checked out this PR and although at first glance it seemed fine, I noticed a bug with it.

In @bounty1342 's implementation, now multiple widgets get the onPressed variable: InkWell and SignInButtonBuilder (which is then passed to MaterialButton). The splash effect indeed works fine when pressing on the middle of the button. The problem is that MaterialButton is bigger than InkWell, so you can press a bit outside of the button and the touch will be passed to MaterialButton but not to InkWell. So the splash bug still remains in that scenario :/

Just switching the material and the inkWell should do the trick.

@ZaynJarvis
Copy link
Owner

I checked out this PR and although at first glance it seemed fine, I noticed a bug with it.
In @bounty1342 's implementation, now multiple widgets get the onPressed variable: InkWell and SignInButtonBuilder (which is then passed to MaterialButton). The splash effect indeed works fine when pressing on the middle of the button. The problem is that MaterialButton is bigger than InkWell, so you can press a bit outside of the button and the touch will be passed to MaterialButton but not to InkWell. So the splash bug still remains in that scenario :/

Just switching the material and the inkWell should do the trick.

could you explain more on this? Maybe fix it with a new commit and resolve the conflicts. Thank you!

@ZaynJarvis
Copy link
Owner

closed since haven't been updated for long

@ZaynJarvis ZaynJarvis closed this Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants