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

Touch ripples are triggered 2 times #1563

Closed
majodev opened this Issue Aug 31, 2015 · 12 comments

Comments

7 participants
@majodev

majodev commented Aug 31, 2015

Looks like touches trigger 2 touch ripple animations: instantly and then with a 300ms delay again. Easily reproducible by visiting the documentation page with an iOS device: http://material-ui.com/#/components/buttons

I'm not sure if Android devices are also affected. Any available workaround for this?

@majodev

This comment has been minimized.

Show comment
Hide comment
@majodev

majodev commented Aug 31, 2015

Might be connected to zilverline/react-tap-event-plugin#14

@mbrookes

This comment has been minimized.

Show comment
Hide comment
@mbrookes

mbrookes Sep 10, 2015

Member

Second this. And further: onClick action only fire with the second ripple, so effectively the 300ms delay is present.

Related(ish): on desktop, even though the ripple only fires once per click, and the onClick action fires straight away, rapidly clicking twice (or more) will trigger the ripple appropriately, but the second onClick action is being lost.

Edit: I needed to use onTouchTap, not onClick - solves the 300ms delay on mobile (event fires on first ripple not second), and also stops events getting dropped on desktop.

Still seeing the double ripple on iOS though per @majodev's originally reported issue.

Member

mbrookes commented Sep 10, 2015

Second this. And further: onClick action only fire with the second ripple, so effectively the 300ms delay is present.

Related(ish): on desktop, even though the ripple only fires once per click, and the onClick action fires straight away, rapidly clicking twice (or more) will trigger the ripple appropriately, but the second onClick action is being lost.

Edit: I needed to use onTouchTap, not onClick - solves the 300ms delay on mobile (event fires on first ripple not second), and also stops events getting dropped on desktop.

Still seeing the double ripple on iOS though per @majodev's originally reported issue.

@ka2n

This comment has been minimized.

Show comment
Hide comment
@ka2n

ka2n Sep 20, 2015

It seems TouchRippe cancels animation when onMouseDown event If other animation was already started by onTouchStart. But in this case, onMouseDown event is fired after onTouchEnd, so it will not be cancelled.

https://github.com/callemall/material-ui/blob/a016c6d850e7d19c652b74d8927e73ae29352058/src/ripples/touch-ripple.jsx#L79

ka2n commented Sep 20, 2015

It seems TouchRippe cancels animation when onMouseDown event If other animation was already started by onTouchStart. But in this case, onMouseDown event is fired after onTouchEnd, so it will not be cancelled.

https://github.com/callemall/material-ui/blob/a016c6d850e7d19c652b74d8927e73ae29352058/src/ripples/touch-ripple.jsx#L79

@seb0zz

This comment has been minimized.

Show comment
Hide comment
@seb0zz

seb0zz Oct 1, 2015

Any update on this?

seb0zz commented Oct 1, 2015

Any update on this?

@majodev

This comment has been minimized.

Show comment
Hide comment
@majodev

majodev Oct 1, 2015

I'm currently "preventing" it by refusing the _handleMouseDown(e) handler to execute while running in a touch environment (cordova) (see https://github.com/callemall/material-ui/blob/master/src/ripples/touch-ripple.jsx#L111). However, this might have some serious side effects...

majodev commented Oct 1, 2015

I'm currently "preventing" it by refusing the _handleMouseDown(e) handler to execute while running in a touch environment (cordova) (see https://github.com/callemall/material-ui/blob/master/src/ripples/touch-ripple.jsx#L111). However, this might have some serious side effects...

@seb0zz

This comment has been minimized.

Show comment
Hide comment
@seb0zz

seb0zz Oct 1, 2015

Ah, yeah this seems to work. Thanks!

seb0zz commented Oct 1, 2015

Ah, yeah this seems to work. Thanks!

@preco21

This comment has been minimized.

Show comment
Hide comment
@preco21

preco21 Oct 11, 2015

Same issue +1

And still double rippled in Chrome 45.0.2454.89, IOS 9.
Test with: http://material-ui.com/#/components/buttons

preco21 commented Oct 11, 2015

Same issue +1

And still double rippled in Chrome 45.0.2454.89, IOS 9.
Test with: http://material-ui.com/#/components/buttons

@mbrookes

This comment has been minimized.

Show comment
Hide comment
@mbrookes

mbrookes Oct 24, 2015

Member

Not just iOS - also seeing this on Android in Chrome and Cordova/crosswalk.

Member

mbrookes commented Oct 24, 2015

Not just iOS - also seeing this on Android in Chrome and Cordova/crosswalk.

@oliviertassinari

This comment has been minimized.

Show comment
Hide comment
@oliviertassinari

oliviertassinari Oct 24, 2015

Member

I have the same result using the touch device emulator of Chrome. I think that we are listening to two different events, those two events fire on a touch device and only one fire on a desktop.

Member

oliviertassinari commented Oct 24, 2015

I have the same result using the touch device emulator of Chrome. I think that we are listening to two different events, those two events fire on a touch device and only one fire on a desktop.

@mbrookes

This comment has been minimized.

Show comment
Hide comment
@mbrookes

mbrookes Oct 24, 2015

Member

@ka2n pointed to the cause in an earlier comment.

Member

mbrookes commented Oct 24, 2015

@ka2n pointed to the cause in an earlier comment.

@oliviertassinari

This comment has been minimized.

Show comment
Hide comment
@oliviertassinari

oliviertassinari Nov 19, 2015

Member

We have just merged a PR of @owencm to fix this issue (#2216)

Member

oliviertassinari commented Nov 19, 2015

We have just merged a PR of @owencm to fix this issue (#2216)

@owencm

This comment has been minimized.

Show comment
Hide comment
@owencm

owencm Nov 23, 2015

Contributor

\o/

Can someone mark this issue as fixed in that case?

Contributor

owencm commented Nov 23, 2015

\o/

Can someone mark this issue as fixed in that case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment