Ripple triggers twice on mobile #646
Conversation
packages/ripple/index.tsx
Outdated
| @@ -242,6 +242,7 @@ export function withRipple < | |||
| }; | |||
|
|
|||
| handleTouchEnd = (e: React.TouchEvent<Surface>) => { | |||
| e.preventDefault(); | |||
There was a problem hiding this comment.
facebook/react#9809 (comment)
touchEnd event causes mouseDown to fire which causes the double ripple on mobile
Codecov Report
@@ Coverage Diff @@
## rc0.10.0 #646 +/- ##
============================================
- Coverage 94.9% 94.77% -0.14%
============================================
Files 68 68
Lines 2847 2833 -14
Branches 428 427 -1
============================================
- Hits 2702 2685 -17
- Misses 50 51 +1
- Partials 95 97 +2
Continue to review full report at Codecov.
|
| @@ -273,7 +274,7 @@ export function withRipple < | |||
| return; | |||
| } | |||
| this.setState((prevState) => { | |||
| const updatedStyle = Object.assign({}, this.state.style) as React.CSSProperties; | |||
| const updatedStyle = Object.assign({}, this.state.style, prevState.style) as React.CSSProperties; | |||
There was a problem hiding this comment.
without requestAnimationFrame wrapping this.foundation.activate(e) previous style is not merged. Adding prevState.style to updatedStyle fixes this.
There was a problem hiding this comment.
Thanks for fixing! This looks fine and it looks like unit tests are passing, however could you remove the rAF from the units tests for the ripple?
ALSO
I ran the screenshot tests and clicked on the anchor tag button. It does not go to a different link like i rc0.10.0. Could you please address that?
|
The anchor tag button is not working because of the preventDefault inside Removing the RAF on all test cases works fine except, the checking of component if it has the |
|
@aluminick we shouldn't use setTimeout when there isn't one in the code. I think we actually need a rAF since there is an rAF in MDC Web's |
|
@aluminick changes look awesome! Please fix |
|
@moog16 no worries. I enjoy debugging. |
|
tests: #659 |
|
@aluminick please sign PR |
|
I signed it |
No description provided.