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

[ClickAwayListener] Handle null child #11955

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .size-limit.js
Original file line number Diff line number Diff line change
@@ -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',
2 changes: 1 addition & 1 deletion packages/material-ui/package.json
Original file line number Diff line number Diff line change
@@ -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"
},
5 changes: 2 additions & 3 deletions packages/material-ui/src/ButtonBase/focusVisible.js
Original file line number Diff line number Diff line change
@@ -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) {
26 changes: 12 additions & 14 deletions packages/material-ui/src/ClickAwayListener/ClickAwayListener.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,27 @@
// @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.
* For instance, if you need to hide a menu when people click anywhere else on your page.
*/
class ClickAwayListener extends React.Component {
componentDidMount() {
this.node = ReactDOM.findDOMNode(this);
this.mounted = true;
}

componentWillUnmount() {
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);
}
Original file line number Diff line number Diff line change
@@ -174,4 +174,20 @@ describe('<ClickAwayListener />', () => {
assert.strictEqual(handleClickAway.callCount, 0);
});
});

it('should hanlde null child', () => {
const Child = () => null;
const handleClickAway = spy();
wrapper = mount(
<ClickAwayListener onClickAway={handleClickAway}>
<Child />
</ClickAwayListener>,
);

const event = document.createEvent('MouseEvents');
event.initEvent('mouseup', true, true);
window.document.body.dispatchEvent(event);

assert.strictEqual(handleClickAway.callCount, 0);
});
});
14 changes: 6 additions & 8 deletions packages/material-ui/src/MenuList/MenuList.js
Original file line number Diff line number Diff line change
@@ -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) {
17 changes: 6 additions & 11 deletions packages/material-ui/src/Modal/Modal.js
Original file line number Diff line number Diff line change
@@ -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,19 +158,17 @@ class Modal extends React.Component {
};

checkForFocus = () => {
if (inDOM) {
this.lastFocus = activeElement();
}
this.lastFocus = ownerDocument(this.mountNode).activeElement;
};

autoFocus() {
if (this.props.disableAutoFocus) {
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();
}
};
5 changes: 2 additions & 3 deletions packages/material-ui/src/Modal/Modal.test.js
Original file line number Diff line number Diff line change
@@ -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('<Modal />', () => {
'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');

2 changes: 1 addition & 1 deletion packages/material-ui/src/Modal/ModalManager.js
Original file line number Diff line number Diff line change
@@ -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';

2 changes: 1 addition & 1 deletion packages/material-ui/src/Modal/isOverflowing.js
Original file line number Diff line number Diff line change
@@ -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) {
5 changes: 2 additions & 3 deletions packages/material-ui/src/Popover/Popover.js
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 1 addition & 1 deletion packages/material-ui/src/Popover/Popover.test.js
Original file line number Diff line number Diff line change
@@ -791,7 +791,7 @@ describe('<Popover />', () => {

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(
<Popover {...defaultProps} getContentAnchorEl={() => child}>
2 changes: 1 addition & 1 deletion packages/material-ui/src/Portal/Portal.js
Original file line number Diff line number Diff line change
@@ -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) {
5 changes: 5 additions & 0 deletions packages/material-ui/src/utils/ownerDocument.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function ownerDocument(node) {
return (node && node.ownerDocument) || document;
}

export default ownerDocument;
6 changes: 3 additions & 3 deletions packages/material-ui/src/utils/ownerWindow.js
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
@@ -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: