From 198173aaa15132a820c9d76f5f580fe4976723ac Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sat, 23 Jun 2018 16:57:17 +0200 Subject: [PATCH] [ClickAwayListener] Handle null child --- .size-limit.js | 6 ++--- packages/material-ui/package.json | 2 +- .../src/ButtonBase/focusVisible.js | 5 ++-- .../ClickAwayListener/ClickAwayListener.js | 26 +++++++++---------- .../ClickAwayListener.test.js | 16 ++++++++++++ packages/material-ui/src/MenuList/MenuList.js | 14 +++++----- packages/material-ui/src/Modal/Modal.js | 17 +++++------- packages/material-ui/src/Modal/Modal.test.js | 5 ++-- .../material-ui/src/Modal/ModalManager.js | 2 +- .../material-ui/src/Modal/isOverflowing.js | 2 +- packages/material-ui/src/Popover/Popover.js | 5 ++-- .../material-ui/src/Popover/Popover.test.js | 2 +- packages/material-ui/src/Portal/Portal.js | 2 +- .../material-ui/src/utils/ownerDocument.js | 5 ++++ packages/material-ui/src/utils/ownerWindow.js | 6 ++--- yarn.lock | 2 +- 16 files changed, 63 insertions(+), 54 deletions(-) create mode 100644 packages/material-ui/src/utils/ownerDocument.js diff --git a/.size-limit.js b/.size-limit.js index a45beca09985dc..e692c652cbcb9f 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -21,19 +21,19 @@ module.exports = [ name: 'The initial cost people pay for using one component', webpack: true, path: 'packages/material-ui/build/Paper/index.js', - limit: '17.8 KB', + limit: '17.6 KB', }, { name: 'The size of all the modules of material-ui.', webpack: true, path: 'packages/material-ui/build/index.js', - limit: '95.0 KB', + limit: '94.6 KB', }, { name: 'The main bundle of the docs', webpack: false, path: getMainFile().path, - limit: '183 KB', + limit: '176 KB', }, { name: 'The home page of the docs', diff --git a/packages/material-ui/package.json b/packages/material-ui/package.json index 083ebf1152c09a..85a3d6a9ca3aa9 100644 --- a/packages/material-ui/package.json +++ b/packages/material-ui/package.json @@ -58,7 +58,7 @@ "react-jss": "^8.1.0", "react-popper": "^0.10.0", "react-transition-group": "^2.2.1", - "recompose": "^0.26.0 || ^0.27.0", + "recompose": "^0.27.0", "scroll": "^2.0.3", "warning": "^4.0.1" }, diff --git a/packages/material-ui/src/ButtonBase/focusVisible.js b/packages/material-ui/src/ButtonBase/focusVisible.js index dcbb770302c50b..4f193e5d2c5046 100644 --- a/packages/material-ui/src/ButtonBase/focusVisible.js +++ b/packages/material-ui/src/ButtonBase/focusVisible.js @@ -2,8 +2,7 @@ import keycode from 'keycode'; import warning from 'warning'; -import contains from 'dom-helpers/query/contains'; -import ownerDocument from 'dom-helpers/ownerDocument'; +import ownerDocument from '../utils/ownerDocument'; const internal = { focusKeyPressed: false, @@ -22,7 +21,7 @@ export function detectFocusVisible(instance, element, callback, attempt = 1) { if ( internal.focusKeyPressed && - (doc.activeElement === element || contains(element, doc.activeElement)) + (doc.activeElement === element || element.contains(doc.activeElement)) ) { callback(); } else if (attempt < instance.focusVisibleMaxCheckTimes) { diff --git a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js index 8db34f31593b4c..e79a3533c54a54 100644 --- a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js +++ b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js @@ -1,18 +1,10 @@ // @inheritedComponent EventListener import React from 'react'; -import PropTypes from 'prop-types'; import ReactDOM from 'react-dom'; +import PropTypes from 'prop-types'; import EventListener from 'react-event-listener'; -import ownerDocument from 'dom-helpers/ownerDocument'; - -function isDescendant(el, target) { - if (target !== null && target.parentNode) { - return el === target || isDescendant(el, target.parentNode); - } - - return false; -} +import ownerDocument from '../utils/ownerDocument'; /** * Listen for click events that occur somewhere in the document, outside of the element itself. @@ -20,6 +12,7 @@ function isDescendant(el, target) { */ class ClickAwayListener extends React.Component { componentDidMount() { + this.node = ReactDOM.findDOMNode(this); this.mounted = true; } @@ -27,7 +20,8 @@ class ClickAwayListener extends React.Component { this.mounted = false; } - mounted = false; + node = null; + mounted = null; handleClickAway = event => { // Ignore events that have been `event.preventDefault()` marked. @@ -40,13 +34,17 @@ class ClickAwayListener extends React.Component { return; } - const el = ReactDOM.findDOMNode(this); - const doc = ownerDocument(el); + // The child might render null. + if (!this.node) { + return; + } + + const doc = ownerDocument(this.node); if ( doc.documentElement && doc.documentElement.contains(event.target) && - !isDescendant(el, event.target) + !this.node.contains(event.target) ) { this.props.onClickAway(event); } diff --git a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.test.js b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.test.js index 9d1a6c2c2479dc..3abb1e37107bc6 100644 --- a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.test.js +++ b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.test.js @@ -174,4 +174,20 @@ describe('', () => { assert.strictEqual(handleClickAway.callCount, 0); }); }); + + it('should hanlde null child', () => { + const Child = () => null; + const handleClickAway = spy(); + wrapper = mount( + + + , + ); + + const event = document.createEvent('MouseEvents'); + event.initEvent('mouseup', true, true); + window.document.body.dispatchEvent(event); + + assert.strictEqual(handleClickAway.callCount, 0); + }); }); diff --git a/packages/material-ui/src/MenuList/MenuList.js b/packages/material-ui/src/MenuList/MenuList.js index 0ea5d552ba2a61..50ff7a305a4ffc 100644 --- a/packages/material-ui/src/MenuList/MenuList.js +++ b/packages/material-ui/src/MenuList/MenuList.js @@ -4,9 +4,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import ReactDOM from 'react-dom'; import keycode from 'keycode'; -import contains from 'dom-helpers/query/contains'; -import activeElement from 'dom-helpers/activeElement'; -import ownerDocument from 'dom-helpers/ownerDocument'; +import ownerDocument from '../utils/ownerDocument'; import List from '../List'; class MenuList extends React.Component { @@ -34,8 +32,8 @@ class MenuList extends React.Component { this.blurTimer = setTimeout(() => { if (this.list) { const list = ReactDOM.findDOMNode(this.list); - const currentFocus = activeElement(ownerDocument(list)); - if (!contains(list, currentFocus)) { + const currentFocus = ownerDocument(list).activeElement; + if (!list.contains(currentFocus)) { this.resetTabIndex(); } } @@ -49,11 +47,11 @@ class MenuList extends React.Component { handleKeyDown = event => { const list = ReactDOM.findDOMNode(this.list); const key = keycode(event); - const currentFocus = activeElement(ownerDocument(list)); + const currentFocus = ownerDocument(list).activeElement; if ( (key === 'up' || key === 'down') && - (!currentFocus || (currentFocus && !contains(list, currentFocus))) + (!currentFocus || (currentFocus && !list.contains(currentFocus))) ) { if (this.selectedItem) { ReactDOM.findDOMNode(this.selectedItem).focus(); @@ -105,7 +103,7 @@ class MenuList extends React.Component { resetTabIndex() { const list = ReactDOM.findDOMNode(this.list); - const currentFocus = activeElement(ownerDocument(list)); + const currentFocus = ownerDocument(list).activeElement; const items = []; for (let i = 0; i < list.children.length; i += 1) { diff --git a/packages/material-ui/src/Modal/Modal.js b/packages/material-ui/src/Modal/Modal.js index 8474814444806f..81de306ba3935d 100644 --- a/packages/material-ui/src/Modal/Modal.js +++ b/packages/material-ui/src/Modal/Modal.js @@ -6,10 +6,7 @@ import PropTypes from 'prop-types'; import classNames from 'classnames'; import warning from 'warning'; import keycode from 'keycode'; -import activeElement from 'dom-helpers/activeElement'; -import contains from 'dom-helpers/query/contains'; -import inDOM from 'dom-helpers/util/inDOM'; -import ownerDocument from 'dom-helpers/ownerDocument'; +import ownerDocument from '../utils/ownerDocument'; import RootRef from '../RootRef'; import Portal from '../Portal'; import { createChainedFunction } from '../utils/helpers'; @@ -161,9 +158,7 @@ class Modal extends React.Component { }; checkForFocus = () => { - if (inDOM) { - this.lastFocus = activeElement(); - } + this.lastFocus = ownerDocument(this.mountNode).activeElement; }; autoFocus() { @@ -171,9 +166,9 @@ class Modal extends React.Component { return; } - const currentActiveElement = activeElement(ownerDocument(this.mountNode)); + const currentActiveElement = ownerDocument(this.mountNode).activeElement; - if (this.dialogElement && !contains(this.dialogElement, currentActiveElement)) { + if (this.dialogElement && !this.dialogElement.contains(currentActiveElement)) { this.lastFocus = currentActiveElement; if (!this.dialogElement.hasAttribute('tabIndex')) { @@ -214,9 +209,9 @@ class Modal extends React.Component { return; } - const currentActiveElement = activeElement(ownerDocument(this.mountNode)); + const currentActiveElement = ownerDocument(this.mountNode).activeElement; - if (this.dialogElement && !contains(this.dialogElement, currentActiveElement)) { + if (this.dialogElement && !this.dialogElement.contains(currentActiveElement)) { this.dialogElement.focus(); } }; diff --git a/packages/material-ui/src/Modal/Modal.test.js b/packages/material-ui/src/Modal/Modal.test.js index 990b6b7da6fcb9..024b87f4da9404 100644 --- a/packages/material-ui/src/Modal/Modal.test.js +++ b/packages/material-ui/src/Modal/Modal.test.js @@ -4,7 +4,6 @@ import React from 'react'; import { assert } from 'chai'; import { spy, stub } from 'sinon'; import keycode from 'keycode'; -import contains from 'dom-helpers/query/contains'; import consoleErrorMock from 'test/utils/consoleErrorMock'; import { createShallow, createMount, getClasses, unwrap } from '../test-utils'; import Fade from '../Fade'; @@ -178,8 +177,8 @@ describe('', () => { 'should have the element in the DOM', ); assert.strictEqual(heading.tagName.toLowerCase(), 'h1', 'should have the element in the DOM'); - assert.strictEqual(contains(portalLayer, container), true, 'should be in the portal'); - assert.strictEqual(contains(portalLayer, heading), true, 'should be in the portal'); + assert.strictEqual(portalLayer.contains(container), true, 'should be in the portal'); + assert.strictEqual(portalLayer.contains(heading), true, 'should be in the portal'); const container2 = document.getElementById('container'); diff --git a/packages/material-ui/src/Modal/ModalManager.js b/packages/material-ui/src/Modal/ModalManager.js index 9756dfbc0b8f70..c969b9f36da5c1 100644 --- a/packages/material-ui/src/Modal/ModalManager.js +++ b/packages/material-ui/src/Modal/ModalManager.js @@ -1,6 +1,6 @@ import css from 'dom-helpers/style'; -import ownerDocument from 'dom-helpers/ownerDocument'; import getScrollbarSize from 'dom-helpers/util/scrollbarSize'; +import ownerDocument from '../utils/ownerDocument'; import isOverflowing from './isOverflowing'; import { ariaHidden, hideSiblings, showSiblings } from './manageAriaHidden'; diff --git a/packages/material-ui/src/Modal/isOverflowing.js b/packages/material-ui/src/Modal/isOverflowing.js index 6ecdeedffafac4..7dc4536f0e955d 100644 --- a/packages/material-ui/src/Modal/isOverflowing.js +++ b/packages/material-ui/src/Modal/isOverflowing.js @@ -1,5 +1,5 @@ import isWindow from 'dom-helpers/query/isWindow'; -import ownerDocument from 'dom-helpers/ownerDocument'; +import ownerDocument from '../utils/ownerDocument'; import ownerWindow from '../utils/ownerWindow'; export function isBody(node) { diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js index da745775db5bac..399c669b8ae04d 100644 --- a/packages/material-ui/src/Popover/Popover.js +++ b/packages/material-ui/src/Popover/Popover.js @@ -4,10 +4,9 @@ import React from 'react'; import PropTypes from 'prop-types'; import ReactDOM from 'react-dom'; import warning from 'warning'; -import contains from 'dom-helpers/query/contains'; -import ownerDocument from 'dom-helpers/ownerDocument'; import debounce from 'debounce'; import EventListener from 'react-event-listener'; +import ownerDocument from '../utils/ownerDocument'; import ownerWindow from '../utils/ownerWindow'; import withStyles from '../styles/withStyles'; import Modal from '../Modal'; @@ -217,7 +216,7 @@ class Popover extends React.Component { if (getContentAnchorEl && anchorReference === 'anchorEl') { const contentAnchorEl = getContentAnchorEl(element); - if (contentAnchorEl && contains(element, contentAnchorEl)) { + if (contentAnchorEl && element.contains(contentAnchorEl)) { const scrollTop = getScrollParent(element, contentAnchorEl); contentAnchorOffset = contentAnchorEl.offsetTop + contentAnchorEl.clientHeight / 2 - scrollTop || 0; diff --git a/packages/material-ui/src/Popover/Popover.test.js b/packages/material-ui/src/Popover/Popover.test.js index b87f87dd1b902a..37356f948e10d2 100644 --- a/packages/material-ui/src/Popover/Popover.test.js +++ b/packages/material-ui/src/Popover/Popover.test.js @@ -791,7 +791,7 @@ describe('', () => { describe('prop: getContentAnchorEl', () => { it('should position accordingly', () => { - const element = { scrollTop: 5 }; + const element = { scrollTop: 5, contains: () => true }; const child = { offsetTop: 40, clientHeight: 20, parentNode: element }; const wrapper = shallow( child}> diff --git a/packages/material-ui/src/Portal/Portal.js b/packages/material-ui/src/Portal/Portal.js index adafa580ee728b..47954e6aed9879 100644 --- a/packages/material-ui/src/Portal/Portal.js +++ b/packages/material-ui/src/Portal/Portal.js @@ -1,7 +1,7 @@ import React from 'react'; import ReactDOM from 'react-dom'; import PropTypes from 'prop-types'; -import ownerDocument from 'dom-helpers/ownerDocument'; +import ownerDocument from '../utils/ownerDocument'; import exactProp from '../utils/exactProp'; function getContainer(container, defaultContainer) { diff --git a/packages/material-ui/src/utils/ownerDocument.js b/packages/material-ui/src/utils/ownerDocument.js new file mode 100644 index 00000000000000..4fabe22e21b10e --- /dev/null +++ b/packages/material-ui/src/utils/ownerDocument.js @@ -0,0 +1,5 @@ +function ownerDocument(node) { + return (node && node.ownerDocument) || document; +} + +export default ownerDocument; diff --git a/packages/material-ui/src/utils/ownerWindow.js b/packages/material-ui/src/utils/ownerWindow.js index 91eda8234909a4..e40c3d828978b6 100644 --- a/packages/material-ui/src/utils/ownerWindow.js +++ b/packages/material-ui/src/utils/ownerWindow.js @@ -1,10 +1,10 @@ // @flow -import ownerDocument from 'dom-helpers/ownerDocument'; +import ownerDocument from '../utils/ownerDocument'; -const ownerWindow = (node: Node, fallback: window = window) => { +function ownerWindow(node: Node, fallback: window = window) { const doc: Document = ownerDocument(node); return doc.defaultView || doc.parentView || fallback; -}; +} export default ownerWindow; diff --git a/yarn.lock b/yarn.lock index f775d549ec3cb5..2ab0ed02aed6d4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8981,7 +8981,7 @@ recast@^0.15.0: private "~0.1.5" source-map "~0.6.1" -"recompose@^0.26.0 || ^0.27.0": +"recompose@^0.26.0 || ^0.27.0", recompose@^0.27.0: version "0.27.1" resolved "https://registry.yarnpkg.com/recompose/-/recompose-0.27.1.tgz#1a49e931f183634516633bbb4f4edbfd3f38a7ba" dependencies: