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

Missing ripple on card #248

Closed
ghost opened this issue Aug 27, 2018 · 9 comments
Closed

Missing ripple on card #248

ghost opened this issue Aug 27, 2018 · 9 comments

Comments

@ghost
Copy link

ghost commented Aug 27, 2018

Hello,

I'm experiencing a problem and I'm not sure if it's something I'm doing wrong or not. I'm using the card component (0.4.2) in a project generated by create react app. When I click on the card, I see the background fade it but the animation for the ripple itself does not appear to fire.

Here is a gif of the behavior I'm seeing:

https://imgur.com/a/3qQkhcW

As you can see, the ripple does not behave in the expected way. As a side note, I have created a clean react application and installed both 0.4.2 and 0.4.1 and both experience this behavior. I have also tried wrapping an element using ripple and it does not work either. The card in rmwc works, however.

Am I doing something wrong or is this a bug in the package?

@moog16
Copy link

moog16 commented Aug 27, 2018

@jcburnette We're missing ripple on the card, and this is a bug. We have a React ripple HOC. You should be able to use that in the meantime until we fix this. We'll get on this bug, but if you figure it out before we fix it, please feel free to open a PR.

Thanks for reporting!

@bmihelac
Copy link
Contributor

If I understand correctly, CardPrimaryContent should be decorated with withRipple. For consistency with MaterialIcon component, new prop hasRipple should be added to CardPrimaryContent to be able to have both variant with and without ripple.

@moog16
Copy link

moog16 commented Aug 29, 2018

@bmihelac that's right, the CardPrimaryContent is the only component we provide in Card that should have the ripple on it. Buttons and icons should too, but the end developer is responsible for using the MDCButton or MaterialIcon to attach the ripple to each individual button or icon.

@moog16
Copy link

moog16 commented Aug 29, 2018

are you gonna fix this one too? :)

@bmihelac
Copy link
Contributor

sure, I'll try

@moog16
Copy link

moog16 commented Aug 29, 2018

sweet thanks!

@moog16
Copy link

moog16 commented Aug 29, 2018

sweet thanks! If you need help please reach out - let me know if you wanna chat too. I can screenshare or hop onto discord as well. If you think the withRipple HOC can be improved, I'm definitely open to hearing that too.

@bmihelac
Copy link
Contributor

sure, will let you know here or on discourse

bmihelac added a commit to bmihelac/material-components-web-react that referenced this issue Aug 30, 2018
bmihelac added a commit to bmihelac/material-components-web-react that referenced this issue Sep 18, 2018
bmihelac added a commit to bmihelac/material-components-web-react that referenced this issue Sep 18, 2018
bmihelac added a commit to bmihelac/material-components-web-react that referenced this issue Sep 19, 2018
@moog16
Copy link

moog16 commented Sep 21, 2018

fixed in #255

@moog16 moog16 closed this as completed Sep 21, 2018
@ghost ghost mentioned this issue Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants