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

fix(ripple): Fix nested ripple handling to work with touch events #2178

Merged
merged 3 commits into from
Feb 5, 2018

Conversation

kfranqueiro
Copy link
Contributor

This reorders code so that the logic that prevents redundant ripple activation from simulated mouse events after a delay on touch devices can function in tandem with the logic which prevents simultaneous activation of nested parent/child ripples.

To see what this fixes, go to list.html, scroll down to the checkbox example, and tap on one of the checkboxes using touch input. The list item should not also ripple at the same time.

I also found and cleaned up a couple of confusing variable declarations (and one excess variable due to one of them).

@codecov-io
Copy link

codecov-io commented Feb 2, 2018

Codecov Report

Merging #2178 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2178   +/-   ##
=======================================
  Coverage   99.14%   99.14%           
=======================================
  Files          90       90           
  Lines        3843     3843           
  Branches      497      497           
=======================================
  Hits         3810     3810           
  Misses         33       33
Impacted Files Coverage Δ
packages/mdc-ripple/foundation.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fe4dcd...8093696. Read the comment docs.

Copy link
Contributor

@patrickrodee patrickrodee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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.

3 participants