fix(ripple): Use mdc-dom.matches everywhere (#4372)

Fixes #4340 `mdc-dom` implements a `matches()` method that wasn't used everywhere. These custom implementations prevented some SSR usage of the material-components library. This commit removes all the custom implementations. I removed some tests that were testing a now removed method, and I handled to make the other pass. PS: This is my first contribution here, I hope I did everything right! BREAKING CHANGE: `getMatchesProperty()` has been removed from `@material/ripple/util` and `@material/tab-scroller/util`. Use `matches()` from `@material/dom/ponyfill` instead.
material-components · Feb 14, 2019 · a2aa3c8 · a2aa3c8
1 parent 3102351
commit a2aa3c8
Show file tree

Hide file tree

Showing 18 changed files with 20 additions and 80 deletions.
diff --git a/packages/mdc-checkbox/index.js b/packages/mdc-checkbox/index.js
@@ -28,7 +28,7 @@ import {MDCSelectionControlState, MDCSelectionControl} from '@material/selection
 /* eslint-enable no-unused-vars */
 import MDCCheckboxFoundation from './foundation';
 import {MDCRipple, MDCRippleFoundation} from '@material/ripple/index';
-import {getMatchesProperty} from '@material/ripple/util';
+import {matches} from '@material/dom/ponyfill';
 
 /** @const {!Array<string>} */
 const CB_PROTO_PROPS = ['checked', 'indeterminate'];
@@ -78,10 +78,9 @@ class MDCCheckbox extends MDCComponent {
    * @private
    */
   initRipple_() {
-    const MATCHES = getMatchesProperty(HTMLElement.prototype);
     const adapter = Object.assign(MDCRipple.createAdapter(this), {
       isUnbounded: () => true,
-      isSurfaceActive: () => this.nativeCb_[MATCHES](':active'),
+      isSurfaceActive: () => matches(/** @type {!Element} */ (this.nativeCb_), ':active'),
       registerInteractionHandler: (type, handler) => this.nativeCb_.addEventListener(type, handler),
       deregisterInteractionHandler: (type, handler) => this.nativeCb_.removeEventListener(type, handler),
     });

diff --git a/packages/mdc-checkbox/package.json b/packages/mdc-checkbox/package.json
@@ -16,6 +16,7 @@
   "dependencies": {
     "@material/animation": "^0.41.0",
     "@material/base": "^0.41.0",
+    "@material/dom": "^0.41.0",
     "@material/feature-targeting": "^0.44.0",
     "@material/ripple": "^0.44.0",
     "@material/rtl": "^0.42.0",

diff --git a/packages/mdc-ripple/README.md b/packages/mdc-ripple/README.md
@@ -304,7 +304,6 @@ Method Signature | Description
 --- | ---
 `util.supportsCssVariables(windowObj, forceRefresh = false) => Boolean` | Determine whether the current browser supports CSS variables (custom properties)
 `util.applyPassive(globalObj = window, forceRefresh = false) => object` | Determine whether the current browser supports passive event listeners
-`util.getMatchesProperty(HTMLElementPrototype) => Function` | Return the correct "matches" property to use on the current browser
 `util.getNormalizedEventCoords(ev, pageOffset, clientRect) => object` | Determines X/Y coordinates of an event normalized for touch events and ripples
 
 > _NOTE_: The functions `util.supportsCssVariables` and `util.applyPassive` cache their results; `forceRefresh` will force recomputation, but is used mainly for testing and should not be necessary in normal use.

diff --git a/packages/mdc-ripple/index.js b/packages/mdc-ripple/index.js
@@ -22,6 +22,7 @@
  */
 
 import MDCComponent from '@material/base/component';
+import {matches} from '@material/dom/ponyfill';
 import MDCRippleAdapter from './adapter';
 import MDCRippleFoundation from './foundation';
 import * as util from './util';
@@ -60,12 +61,10 @@ class MDCRipple extends MDCComponent {
    * @return {!MDCRippleAdapter}
    */
   static createAdapter(instance) {
-    const MATCHES = util.getMatchesProperty(HTMLElement.prototype);
-
     return {
       browserSupportsCssVars: () => util.supportsCssVariables(window),
       isUnbounded: () => instance.unbounded,
-      isSurfaceActive: () => instance.root_[MATCHES](':active'),
+      isSurfaceActive: () => matches(instance.root_, ':active'),
       isSurfaceDisabled: () => instance.disabled,
       addClass: (className) => instance.root_.classList.add(className),
       removeClass: (className) => instance.root_.classList.remove(className),

diff --git a/packages/mdc-ripple/package.json b/packages/mdc-ripple/package.json
@@ -16,6 +16,7 @@
   "dependencies": {
     "@material/animation": "^0.41.0",
     "@material/base": "^0.41.0",
+    "@material/dom": "^0.41.0",
     "@material/feature-targeting": "^0.44.0",
     "@material/theme": "^0.43.0"
   }

diff --git a/packages/mdc-ripple/util.js b/packages/mdc-ripple/util.js
@@ -117,28 +117,6 @@ function applyPassive(globalObj = window, forceRefresh = false) {
     : false;
 }
 
-/**
- * @param {!Object} HTMLElementPrototype
- * @return {string}
- */
-function getMatchesProperty(HTMLElementPrototype) {
-  /**
-   * Order is important because we return the first existing method we find.
-   * Do not change the order of the items in the below array.
-   */
-  const matchesMethods = ['matches', 'webkitMatchesSelector', 'msMatchesSelector'];
-  let method = 'matches';
-  for (let i = 0; i < matchesMethods.length; i++) {
-    const matchesMethod = matchesMethods[i];
-    if (matchesMethod in HTMLElementPrototype) {
-      method = matchesMethod;
-      break;
-    }
-  }
-
-  return method;
-}
-
 /**
  * @param {!Event} ev
  * @param {{x: number, y: number}} pageOffset
@@ -166,4 +144,4 @@ function getNormalizedEventCoords(ev, pageOffset, clientRect) {
   return {x: normalizedX, y: normalizedY};
 }
 
-export {supportsCssVariables, applyPassive, getMatchesProperty, getNormalizedEventCoords};
+export {supportsCssVariables, applyPassive, getNormalizedEventCoords};
diff --git a/packages/mdc-switch/index.js b/packages/mdc-switch/index.js
@@ -27,7 +27,7 @@ import {MDCSelectionControlState, MDCSelectionControl} from '@material/selection
 /* eslint-enable no-unused-vars */
 import MDCSwitchFoundation from './foundation';
 import {MDCRipple, MDCRippleFoundation} from '@material/ripple/index';
-import {getMatchesProperty} from '@material/ripple/util';
+import {matches} from '@material/dom/ponyfill';
 
 /**
  * @extends MDCComponent<!MDCSwitchFoundation>
@@ -84,10 +84,9 @@ class MDCSwitch extends MDCComponent {
     const {RIPPLE_SURFACE_SELECTOR} = MDCSwitchFoundation.strings;
     const rippleSurface = /** @type {!Element} */ (this.root_.querySelector(RIPPLE_SURFACE_SELECTOR));
 
-    const MATCHES = getMatchesProperty(HTMLElement.prototype);
     const adapter = Object.assign(MDCRipple.createAdapter(this), {
       isUnbounded: () => true,
-      isSurfaceActive: () => this.nativeControl_[MATCHES](':active'),
+      isSurfaceActive: () => matches(/** @type {!Element} */ (this.nativeControl_), ':active'),
       addClass: (className) => rippleSurface.classList.add(className),
       removeClass: (className) => rippleSurface.classList.remove(className),
       registerInteractionHandler: (type, handler) => this.nativeControl_.addEventListener(type, handler),

diff --git a/packages/mdc-switch/package.json b/packages/mdc-switch/package.json
@@ -16,6 +16,7 @@
   "dependencies": {
     "@material/animation": "^0.41.0",
     "@material/base": "^0.41.0",
+    "@material/dom": "^0.41.0",
     "@material/elevation": "^0.44.0",
     "@material/feature-targeting": "^0.44.0",
     "@material/ripple": "^0.44.0",

diff --git a/packages/mdc-tab-scroller/README.md b/packages/mdc-tab-scroller/README.md
@@ -108,7 +108,6 @@ MDC Tab Scroller provides a `util` module with functions to help implement adapt
 Function Signature | Description
 --- | ---
 `computeHorizontalScrollbarHeight(document: Document) => number` | Returns the height of the browser's horizontal scrollbars (in px).
-`getMatchesProperty(HTMLElementPrototype: Object) => string` | Returns the appropriate property name for the `matches` API in the current browser environment.
 
 ### `MDCTabScrollerFoundation`
 

diff --git a/packages/mdc-tab-scroller/index.js b/packages/mdc-tab-scroller/index.js
@@ -22,6 +22,7 @@
  */
 
 import MDCComponent from '@material/base/component';
+import {matches} from '@material/dom/ponyfill';
 
 import {MDCTabScrollerAdapter} from './adapter';
 import MDCTabScrollerFoundation from './foundation';
@@ -90,8 +91,7 @@ class MDCTabScroller extends MDCComponent {
   getDefaultFoundation() {
     const adapter = /** @type {!MDCTabScrollerAdapter} */ ({
       eventTargetMatchesSelector: (evtTarget, selector) => {
-        const MATCHES = util.getMatchesProperty(HTMLElement.prototype);
-        return evtTarget[MATCHES](selector);
+        return matches(evtTarget, selector);
       },
       addClass: (className) => this.root_.classList.add(className),
       removeClass: (className) => this.root_.classList.remove(className),

diff --git a/packages/mdc-tab-scroller/package.json b/packages/mdc-tab-scroller/package.json
@@ -17,6 +17,7 @@
   "dependencies": {
     "@material/animation": "^0.41.0",
     "@material/base": "^0.41.0",
+    "@material/dom": "^0.41.0",
     "@material/tab": "^0.44.0"
   },
   "publishConfig": {

diff --git a/packages/mdc-tab-scroller/util.js b/packages/mdc-tab-scroller/util.js
@@ -54,14 +54,4 @@ function computeHorizontalScrollbarHeight(documentObj, shouldCacheResult = true)
   return horizontalScrollbarHeight;
 }
 
-/**
- * @param {!Object} HTMLElementPrototype
- * @return {string}
- */
-function getMatchesProperty(HTMLElementPrototype) {
-  return [
-    'msMatchesSelector', 'matches',
-  ].filter((p) => p in HTMLElementPrototype).pop();
-}
-
-export {computeHorizontalScrollbarHeight, getMatchesProperty};
+export {computeHorizontalScrollbarHeight};
diff --git a/packages/mdc-textfield/index.js b/packages/mdc-textfield/index.js
@@ -25,7 +25,7 @@ import MDCComponent from '@material/base/component';
 /* eslint-disable no-unused-vars */
 import {MDCRipple, MDCRippleFoundation, RippleCapableSurface} from '@material/ripple/index';
 /* eslint-enable no-unused-vars */
-import {getMatchesProperty} from '@material/ripple/util';
+import {matches} from '@material/dom/ponyfill';
 
 
 import {cssClasses, strings} from './constants';
@@ -153,10 +153,9 @@ class MDCTextField extends MDCComponent {
 
     this.ripple = null;
     if (!this.root_.classList.contains(cssClasses.TEXTAREA) && !this.root_.classList.contains(cssClasses.OUTLINED)) {
-      const MATCHES = getMatchesProperty(HTMLElement.prototype);
       const adapter =
         Object.assign(MDCRipple.createAdapter(/** @type {!RippleCapableSurface} */ (this)), {
-          isSurfaceActive: () => this.input_[MATCHES](':active'),
+          isSurfaceActive: () => matches(/** @type {!Element} */ (this.input_), ':active'),
           registerInteractionHandler: (type, handler) => this.input_.addEventListener(type, handler),
           deregisterInteractionHandler: (type, handler) => this.input_.removeEventListener(type, handler),
         });

diff --git a/packages/mdc-textfield/package.json b/packages/mdc-textfield/package.json
@@ -17,6 +17,7 @@
   "dependencies": {
     "@material/animation": "^0.41.0",
     "@material/base": "^0.41.0",
+    "@material/dom": "^0.41.0",
     "@material/floating-label": "^0.44.0",
     "@material/line-ripple": "^0.43.0",
     "@material/notched-outline": "^0.44.0",

diff --git a/test/unit/mdc-checkbox/mdc-checkbox.test.js b/test/unit/mdc-checkbox/mdc-checkbox.test.js
@@ -31,7 +31,6 @@ import {supportsCssVariables} from '../../../packages/mdc-ripple/util';
 import {MDCCheckbox, MDCCheckboxFoundation} from '../../../packages/mdc-checkbox/index';
 import {MDCRipple} from '../../../packages/mdc-ripple/index';
 import {strings} from '../../../packages/mdc-checkbox/constants';
-import {getMatchesProperty} from '../../../packages/mdc-ripple/util';
 
 function getFixture() {
   return bel`
@@ -97,7 +96,9 @@ if (supportsCssVariables(window)) {
 
     const fakeMatches = td.func('.matches');
     td.when(fakeMatches(':active')).thenReturn(true);
-    input[getMatchesProperty(HTMLElement.prototype)] = fakeMatches;
+    input.matches = fakeMatches;
+    input.webkitMatchesSelector = fakeMatches;
+    input.msMatchesSelector = fakeMatches;
 
     assert.isTrue(root.classList.contains('mdc-ripple-upgraded'));
     domEvents.emit(input, 'keydown');

diff --git a/test/unit/mdc-ripple/mdc-ripple.test.js b/test/unit/mdc-ripple/mdc-ripple.test.js
@@ -117,15 +117,6 @@ test('adapter#isUnbounded delegates to unbounded getter', () => {
   assert.isOk(component.getDefaultFoundation().adapter_.isUnbounded());
 });
 
-test('adapter#isSurfaceActive calls the correct :matches API method on the root element', () => {
-  const {root, component} = setupTest();
-  const MATCHES = util.getMatchesProperty(HTMLElement.prototype);
-  const matches = td.func('root.<matches>');
-  td.when(matches(':active')).thenReturn(true);
-  root[MATCHES] = matches;
-  assert.isOk(component.getDefaultFoundation().adapter_.isSurfaceActive());
-});
-
 test('adapter#isSurfaceDisabled delegates to component\'s disabled getter', () => {
   const {component} = setupTest();
   component.disabled = true;

diff --git a/test/unit/mdc-ripple/util.test.js b/test/unit/mdc-ripple/util.test.js
@@ -117,16 +117,6 @@ test('applyPassive returns false for browsers that do not support passive event
   assert.isNotOk(util.applyPassive(mockWindow, true));
 });
 
-test('#getMatchesProperty returns the correct property for selector matching', () => {
-  assert.equal(util.getMatchesProperty({matches: () => {}}), 'matches');
-  assert.equal(util.getMatchesProperty({webkitMatchesSelector: () => {}}), 'webkitMatchesSelector');
-  assert.equal(util.getMatchesProperty({msMatchesSelector: () => {}}), 'msMatchesSelector');
-});
-
-test('#getMatchesProperty returns the standard function if more than one method is present', () => {
-  assert.equal(util.getMatchesProperty({matches: () => {}, webkitMatchesSelector: () => {}}), 'matches');
-});
-
 test('#getNormalizedEventCoords maps event coords into the relative coordinates of the given rect', () => {
   const ev = {type: 'mousedown', pageX: 70, pageY: 70};
   const pageOffset = {x: 10, y: 10};

diff --git a/test/unit/mdc-tab-scroller/util.test.js b/test/unit/mdc-tab-scroller/util.test.js
@@ -58,12 +58,3 @@ test('#computeHorizontalScrollbarHeight returns value based on difference betwee
     expectedHeight);
   td.verify(classListAddFunc(cssClasses.SCROLL_TEST));
 });
-
-test('#getMatchesProperty returns the correct property for selector matching', () => {
-  assert.strictEqual(util.getMatchesProperty({matches: () => {}}), 'matches');
-  assert.strictEqual(util.getMatchesProperty({msMatchesSelector: () => {}}), 'msMatchesSelector');
-});
-
-test('#getMatchesProperty returns the standard function if more than one method is present', () => {
-  assert.strictEqual(util.getMatchesProperty({matches: () => {}, msMatchesSelector: () => {}}), 'matches');
-});