fix(ripple): Only deduplicate events on parents whose children activated (#2160)

BREAKING CHANGE: Adds `containsEventTarget(target)` API to the ripple adapter.

Author: kfranqueiro

packages/mdc-ripple/README.md

@@ -153,6 +153,7 @@ Method Signature | Description
 | `isSurfaceDisabled() => boolean` | Whether or not the ripple is attached to a disabled component |
 | `addClass(className: string) => void` | Adds a class to the ripple surface |
 | `removeClass(className: string) => void` | Removes a class from the ripple surface |
+| `containsEventTarget(target: EventTarget) => boolean` | Whether or not the ripple surface contains the given event target |
 | `registerInteractionHandler(evtType: string, handler: EventListener) => void` | Registers an event handler on the ripple surface |
 | `deregisterInteractionHandler(evtType: string, handler: EventListener) => void` | Unregisters an event handler on the ripple surface |
 | `registerDocumentInteractionHandler(evtType: string, handler: EventListener) => void` | Registers an event handler on the documentElement |

packages/mdc-ripple/adapter.js

@@ -57,6 +57,9 @@ class MDCRippleAdapter {
   /** @param {string} className */
   removeClass(className) {}
 
+  /** @param {!EventTarget} target */
+  containsEventTarget(target) {}
+
   /**
    * @param {string} evtType
    * @param {!Function} handler

packages/mdc-ripple/foundation.js

@@ -66,8 +66,9 @@ const ACTIVATION_EVENT_TYPES = ['touchstart', 'pointerdown', 'mousedown', 'keydo
 // Deactivation events registered on documentElement when a pointer-related down event occurs
 const POINTER_DEACTIVATION_EVENT_TYPES = ['touchend', 'pointerup', 'mouseup'];
 
-// Tracks whether an activation has occurred on the current frame, to avoid multiple nested activations
-let isActivating = false;
+// Tracks activations that have occurred on the current frame, to avoid simultaneous nested activations
+/** @type {!Array<!EventTarget>} */
+let activatedTargets = [];
 
 /**
  * @extends {MDCFoundation<!MDCRippleAdapter>}
@@ -93,6 +94,7 @@ class MDCRippleFoundation extends MDCFoundation {
       isSurfaceDisabled: () => /* boolean */ {},
       addClass: (/* className: string */) => {},
       removeClass: (/* className: string */) => {},
+      containsEventTarget: (/* target: !EventTarget */) => {},
       registerInteractionHandler: (/* evtType: string, handler: EventListener */) => {},
       deregisterInteractionHandler: (/* evtType: string, handler: EventListener */) => {},
       registerDocumentInteractionHandler: (/* evtType: string, handler: EventListener */) => {},
@@ -284,7 +286,13 @@ class MDCRippleFoundation extends MDCFoundation {
    * @private
    */
   activate_(e) {
-    if (isActivating || this.adapter_.isSurfaceDisabled()) {
+    if (this.adapter_.isSurfaceDisabled()) {
+      return;
+    }
+
+    const hasActivatedChild =
+      e && activatedTargets.length > 0 && activatedTargets.some((target) => this.adapter_.containsEventTarget(target));
+    if (hasActivatedChild) {
       return;
     }
 
@@ -308,10 +316,10 @@ class MDCRippleFoundation extends MDCFoundation {
     );
 
     if (e) {
+      activatedTargets.push(/** @type {!EventTarget} */ (e.target));
       this.registerDeactivationHandlers_(e);
     }
 
-    isActivating = true;
     requestAnimationFrame(() => {
       // This needs to be wrapped in an rAF call b/c web browsers
       // report active states inconsistently when they're called within
@@ -326,8 +334,8 @@ class MDCRippleFoundation extends MDCFoundation {
         this.activationState_ = this.defaultActivationState_();
       }
 
-      // Reset flag on next frame to avoid any ancestors from also triggering ripple from the same interaction
-      isActivating = false;
+      // Reset array on next frame after the current event has had a chance to bubble to prevent ancestor ripples
+      activatedTargets = [];
     });
   }
 

packages/mdc-ripple/index.js

@@ -63,6 +63,7 @@ class MDCRipple extends MDCComponent {
       isSurfaceDisabled: () => instance.disabled,
       addClass: (className) => instance.root_.classList.add(className),
       removeClass: (className) => instance.root_.classList.remove(className),
+      containsEventTarget: (target) => instance.root_.contains(target),
       registerInteractionHandler: (evtType, handler) =>
         instance.root_.addEventListener(evtType, handler, util.applyPassive()),
       deregisterInteractionHandler: (evtType, handler) =>

test/unit/mdc-ripple/foundation-activation.test.js

@@ -318,17 +318,16 @@ testFoundation('removes deactivation classes on activate to ensure ripples can b
     td.verify(adapter.removeClass(cssClasses.FG_DEACTIVATION));
   });
 
-testFoundation('will not activate multiple ripples on same frame',
+testFoundation('will not activate multiple ripples on same frame if one surface descends from another',
   ({foundation, adapter, mockRaf}) => {
     const secondRipple = setupTest();
     const firstHandlers = captureHandlers(adapter, 'registerInteractionHandler');
     const secondHandlers = captureHandlers(secondRipple.adapter, 'registerInteractionHandler');
+    td.when(secondRipple.adapter.containsEventTarget(td.matchers.anything())).thenReturn(true);
     foundation.init();
     secondRipple.foundation.init();
     mockRaf.flush();
 
-    // Simulate use case where a child and parent are both ripple surfaces, and the same event propagates to the
-    // parent after being handled on the child
     firstHandlers.mousedown();
     secondHandlers.mousedown();
     mockRaf.flush();
@@ -337,6 +336,24 @@ testFoundation('will not activate multiple ripples on same frame',
     td.verify(secondRipple.adapter.addClass(cssClasses.FG_ACTIVATION), {times: 0});
   });
 
+testFoundation('will activate multiple ripples on same frame for surfaces without an ancestor/descendant relationship',
+  ({foundation, adapter, mockRaf}) => {
+    const secondRipple = setupTest();
+    const firstHandlers = captureHandlers(adapter, 'registerInteractionHandler');
+    const secondHandlers = captureHandlers(secondRipple.adapter, 'registerInteractionHandler');
+    td.when(secondRipple.adapter.containsEventTarget(td.matchers.anything())).thenReturn(false);
+    foundation.init();
+    secondRipple.foundation.init();
+    mockRaf.flush();
+
+    firstHandlers.mousedown();
+    secondHandlers.mousedown();
+    mockRaf.flush();
+
+    td.verify(adapter.addClass(cssClasses.FG_ACTIVATION));
+    td.verify(secondRipple.adapter.addClass(cssClasses.FG_ACTIVATION));
+  });
+
 testFoundation('displays the foreground ripple on activation when unbounded', ({foundation, adapter, mockRaf}) => {
   const handlers = captureHandlers(adapter, 'registerInteractionHandler');
   td.when(adapter.computeBoundingRect()).thenReturn({width: 100, height: 100, left: 0, top: 0});

test/unit/mdc-ripple/foundation.test.js

@@ -40,7 +40,7 @@ test('numbers returns constants.numbers', () => {
 test('defaultAdapter returns a complete adapter implementation', () => {
   verifyDefaultAdapter(MDCRippleFoundation, [
     'browserSupportsCssVars', 'isUnbounded', 'isSurfaceActive', 'isSurfaceDisabled',
-    'addClass', 'removeClass', 'registerInteractionHandler', 'deregisterInteractionHandler',
+    'addClass', 'removeClass', 'containsEventTarget', 'registerInteractionHandler', 'deregisterInteractionHandler',
     'registerDocumentInteractionHandler', 'deregisterDocumentInteractionHandler',
     'registerResizeHandler', 'deregisterResizeHandler', 'updateCssVariable',
     'computeBoundingRect', 'getWindowPageOffset',

test/unit/mdc-ripple/mdc-ripple.test.js

@@ -137,6 +137,15 @@ test('adapter#removeClass removes a class from the root', () => {
   assert.isNotOk(root.classList.contains('foo'));
 });
 
+test('adapter#containsEventTarget returns true if the passed element is a descendant of the root element', () => {
+  const {root, component} = setupTest();
+  const child = bel`<div></div>`;
+  const notChild = bel`<div></div>`;
+  root.appendChild(child);
+  assert.isTrue(component.getDefaultFoundation().adapter_.containsEventTarget(child));
+  assert.isFalse(component.getDefaultFoundation().adapter_.containsEventTarget(notChild));
+});
+
 test('adapter#registerInteractionHandler proxies to addEventListener on the root element', () => {
   const {root, component} = setupTest();
   const handler = td.func('interactionHandler');
@@ -180,7 +189,7 @@ test('adapter#registerResizeHandler uses the handler as a window resize listener
   window.removeEventListener('resize', handler);
 });
 
-test('adapter#registerResizeHandler unlistens the handler for window resize', () => {
+test('adapter#deregisterResizeHandler unlistens the handler for window resize', () => {
   const {component} = setupTest();
   const handler = td.func('resizeHandler');
   window.addEventListener('resize', handler);