fix(ripple): Only deduplicate ripple on parents whose children activated #2160
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2160 +/- ##
==========================================
+ Coverage 99.14% 99.14% +<.01%
==========================================
Files 90 90
Lines 3840 3845 +5
Branches 495 497 +2
==========================================
+ Hits 3807 3812 +5
Misses 33 33
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This looks good but I'm curious if we could just move the isActivating flag onto the ripple foundation as a private instance boolean that we toggle just like before, preventing any foundation from being activated twice on the same frame, regardless of what event target activated it. But... that completely precludes the possibility of ripples on parent and children nodes which IDK if that's supposed to be possible.... Anyway, LGTM!
|
Discussed with Patrick offline; the reason this tracks across instances is in order to avoid 2 ripples happening from the same single user interaction due to both existing in the same DOM hierarchy, so we need to track this across instances. This amends the original fix which simply ignored subsequent calls on the same frame, which had potential to ignore more events than it should. |
This amends #2123 to check for parent/child relationship before suppressing multiple ripples on the same frame, which should allow for rare cases where it makes sense for ripples to happen on multiple unrelated surfaces at the same time (e.g. multitouch, and also helps for screenshot testing).
Checked timing on all browsers this JS loads for (non-IE/Edge) and it takes less than 1ms (generally much less in Chrome/Safari).
BREAKING CHANGE: Adds
containsEventTarget(target)API to the ripple adapter.