From 3a69cb4f4068f14f61eba9648c7d586cf13975dc Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Fri, 15 Feb 2019 17:18:01 +0100 Subject: [PATCH] [Modal] Improve the focus logic --- .size-limit.js | 11 +- docs/src/pages/demos/dialogs/dialogs.md | 2 +- docs/src/pages/utils/modal/modal.md | 21 +++ packages/material-ui/src/Menu/Menu.js | 2 +- packages/material-ui/src/Modal/Modal.js | 85 ++-------- packages/material-ui/src/Modal/Modal.test.js | 8 +- .../material-ui/src/Modal/ModalManager.js | 2 +- packages/material-ui/src/Modal/TrapFocus.js | 145 ++++++++++++++++++ 8 files changed, 195 insertions(+), 81 deletions(-) create mode 100644 packages/material-ui/src/Modal/TrapFocus.js diff --git a/.size-limit.js b/.size-limit.js index 3d559c59161ad2..2dedad80022275 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -66,7 +66,7 @@ module.exports = [ name: 'The size of the @material-ui/core/Modal component', webpack: true, path: 'packages/material-ui/build/esm/Modal/index.js', - limit: '24.1 KB', + limit: '24.3 KB', }, { // vs https://bundlephobia.com/result?p=react-popper @@ -75,6 +75,13 @@ module.exports = [ path: 'packages/material-ui/build/esm/Popper/index.js', limit: '9.8 KB', }, + { + // vs https://bundlephobia.com/result?p=focus-trap-react + name: 'The size of the @material-ui/core/Popper component', + webpack: true, + path: 'packages/material-ui/build/esm/Modal/TrapFocus.js', + limit: '1.6 KB', + }, { // vs https://bundlephobia.com/result?p=react-responsive // vs https://bundlephobia.com/result?p=react-media @@ -87,7 +94,7 @@ module.exports = [ name: 'The main docs bundle', webpack: false, path: main.path, - limit: '202 KB', + limit: '203 KB', }, { name: 'The docs home page', diff --git a/docs/src/pages/demos/dialogs/dialogs.md b/docs/src/pages/demos/dialogs/dialogs.md index b26276b8b07518..a72046a15d9aea 100644 --- a/docs/src/pages/demos/dialogs/dialogs.md +++ b/docs/src/pages/demos/dialogs/dialogs.md @@ -92,7 +92,7 @@ Touching “Cancel” in a confirmation dialog, or pressing Back, cancels the ac ## Accessibility -Be sure to add `aria-labelledby="id..."`, referencing the modal title, to the `Dialog`. Additionally, you may give a description of your modal dialog with the `aria-describedby="id..."` property on the `Dialog`. +Follow the [Modal accessibility section](/utils/modal/#accessibility). ## Scrolling long content diff --git a/docs/src/pages/utils/modal/modal.md b/docs/src/pages/utils/modal/modal.md index 6a808540384c9f..4a00cf23f1fdfe 100644 --- a/docs/src/pages/utils/modal/modal.md +++ b/docs/src/pages/utils/modal/modal.md @@ -77,3 +77,24 @@ You can **speed up** the rendering by moving the modal body into its own compone This way, you take advantage of [React render laziness evaluation](https://overreacted.io/react-as-a-ui-runtime/#lazy-evaluation). The `TableComponent` render method will only be evaluated when opening the modal. + +## Accessibility + +- Be sure to add `aria-labelledby="id..."`, referencing the modal title, to the `Modal`. +Additionally, you may give a description of your modal with the `aria-describedby="id..."` property on the `Modal`. + +```jsx + + + My Title + + + My Description + + +``` + +- The [WAI-ARIA Authoring Practices 1.1](https://www.w3.org/TR/wai-aria-practices/examples/dialog-modal/dialog.html) can help you set the initial focus on the most relevant element, based on your modal content. diff --git a/packages/material-ui/src/Menu/Menu.js b/packages/material-ui/src/Menu/Menu.js index d5395922e1945f..aade9c13a4b852 100644 --- a/packages/material-ui/src/Menu/Menu.js +++ b/packages/material-ui/src/Menu/Menu.js @@ -73,7 +73,7 @@ class Menu extends React.Component { // Let's ignore that piece of logic if users are already overriding the width // of the menu. if (menuList && element.clientHeight < menuList.clientHeight && !menuList.style.width) { - const size = `${getScrollbarSize()}px`; + const size = `${getScrollbarSize(true)}px`; menuList.style[theme.direction === 'rtl' ? 'paddingLeft' : 'paddingRight'] = size; menuList.style.width = `calc(100% + ${size})`; } diff --git a/packages/material-ui/src/Modal/Modal.js b/packages/material-ui/src/Modal/Modal.js index c3402cea746e9f..4bb117c372dce8 100644 --- a/packages/material-ui/src/Modal/Modal.js +++ b/packages/material-ui/src/Modal/Modal.js @@ -2,14 +2,13 @@ import React from 'react'; import ReactDOM from 'react-dom'; import PropTypes from 'prop-types'; import clsx from 'clsx'; -import warning from 'warning'; import { componentPropType } from '@material-ui/utils'; import ownerDocument from '../utils/ownerDocument'; -import RootRef from '../RootRef'; import Portal from '../Portal'; import { createChainedFunction } from '../utils/helpers'; import withStyles from '../styles/withStyles'; import ModalManager from './ModalManager'; +import TrapFocus from './TrapFocus'; import Backdrop from '../Backdrop'; import { ariaHidden } from './manageAriaHidden'; @@ -107,9 +106,7 @@ class Modal extends React.Component { const container = getContainer(this.props.container, doc.body); this.props.manager.add(this, container); - doc.addEventListener('focus', this.enforceFocus, true); - - if (this.dialogRef) { + if (this.modalRef) { this.handleOpened(); } }; @@ -127,7 +124,6 @@ class Modal extends React.Component { }; handleOpened = () => { - this.autoFocus(); this.props.manager.mount(this); // Fix a bug on Chrome where the scroll isn't initially 0. @@ -142,11 +138,6 @@ class Modal extends React.Component { if (!(hasTransition && this.props.closeAfterTransition) || reason === 'unmount') { this.props.manager.remove(this); } - - const doc = ownerDocument(this.mountNode); - doc.removeEventListener('focus', this.enforceFocus, true); - - this.restoreLastFocus(); }; handleExited = () => { @@ -196,19 +187,6 @@ class Modal extends React.Component { } }; - enforceFocus = () => { - // The Modal might not already be mounted. - if (!this.isTopModal() || this.props.disableEnforceFocus || !this.mounted || !this.dialogRef) { - return; - } - - const currentActiveElement = ownerDocument(this.mountNode).activeElement; - - if (!this.dialogRef.contains(currentActiveElement)) { - this.dialogRef.focus(); - } - }; - handlePortalRef = ref => { this.mountNode = ref ? ref.getMountNode() : ref; }; @@ -217,54 +195,9 @@ class Modal extends React.Component { this.modalRef = ref; }; - onRootRef = ref => { - this.dialogRef = ref; - }; - - autoFocus() { - // We might render an empty child. - if (this.props.disableAutoFocus || !this.dialogRef) { - return; - } - - const currentActiveElement = ownerDocument(this.mountNode).activeElement; - - if (!this.dialogRef.contains(currentActiveElement)) { - if (!this.dialogRef.hasAttribute('tabIndex')) { - warning( - false, - [ - 'Material-UI: the modal content node does not accept focus.', - 'For the benefit of assistive technologies, ' + - 'the tabIndex of the node is being set to "-1".', - ].join('\n'), - ); - this.dialogRef.setAttribute('tabIndex', -1); - } - - this.lastFocus = currentActiveElement; - this.dialogRef.focus(); - } - } - - restoreLastFocus() { - if (this.props.disableRestoreFocus || !this.lastFocus) { - return; - } - - // Not all elements in IE 11 have a focus method. - // Because IE 11 market share is low, we accept the restore focus being broken - // and we silent the issue. - if (this.lastFocus.focus) { - this.lastFocus.focus(); - } - - this.lastFocus = null; - } - - isTopModal() { + isTopModal = () => { return this.props.manager.isTopModal(this); - } + }; render() { const { @@ -339,7 +272,15 @@ class Modal extends React.Component { {hideBackdrop ? null : ( )} - {React.cloneElement(children, childProps)} + + {React.cloneElement(children, childProps)} + ); diff --git a/packages/material-ui/src/Modal/Modal.test.js b/packages/material-ui/src/Modal/Modal.test.js index 3440d1a7aff480..83598d52a6419e 100644 --- a/packages/material-ui/src/Modal/Modal.test.js +++ b/packages/material-ui/src/Modal/Modal.test.js @@ -218,9 +218,9 @@ describe('', () => { throw new Error('missing modal'); } - assert.strictEqual(modal.children.length, 2); + assert.strictEqual(modal.children.length, 4); assert.strictEqual(modal.children[0] != null, true); - assert.strictEqual(modal.children[1], container); + assert.strictEqual(modal.children[2], container); }); }); @@ -240,8 +240,8 @@ describe('', () => { throw new Error('missing modal'); } - assert.strictEqual(modal.children.length, 1); - assert.strictEqual(modal.children[0], container); + assert.strictEqual(modal.children.length, 3); + assert.strictEqual(modal.children[1], container); }); }); diff --git a/packages/material-ui/src/Modal/ModalManager.js b/packages/material-ui/src/Modal/ModalManager.js index 3aa822fb937a30..c75f3c1ea749d8 100644 --- a/packages/material-ui/src/Modal/ModalManager.js +++ b/packages/material-ui/src/Modal/ModalManager.js @@ -32,7 +32,7 @@ function setContainerStyle(data) { }; if (data.overflowing) { - const scrollbarSize = getScrollbarSize(); + const scrollbarSize = getScrollbarSize(true); // Use computed style, here to get the real padding to add our scrollbar width. style.paddingRight = `${getPaddingRight(data.container) + scrollbarSize}px`; diff --git a/packages/material-ui/src/Modal/TrapFocus.js b/packages/material-ui/src/Modal/TrapFocus.js new file mode 100644 index 00000000000000..8ae03bf0c99994 --- /dev/null +++ b/packages/material-ui/src/Modal/TrapFocus.js @@ -0,0 +1,145 @@ +/* eslint-disable consistent-return, jsx-a11y/no-noninteractive-tabindex */ + +import React from 'react'; +import PropTypes from 'prop-types'; +import warning from 'warning'; +import RootRef from '../RootRef'; +import ownerDocument from '../utils/ownerDocument'; + +function TrapFocus(props) { + const { disableEnforceFocus, disableAutoFocus, disableRestoreFocus, isEnabled, open } = props; + const rootRef = React.useRef(); + const ignoreNextEnforceFocus = React.useRef(); + const sentinelStart = React.useRef(); + const sentinelEnd = React.useRef(); + const lastFocus = React.useRef(); + + React.useEffect(() => { + if (!open) { + return; + } + + const doc = ownerDocument(rootRef.current); + const currentActiveElement = doc.activeElement; + lastFocus.current = currentActiveElement; + + // We might render an empty child. + if (!disableAutoFocus && rootRef.current && !rootRef.current.contains(currentActiveElement)) { + if (!rootRef.current.hasAttribute('tabIndex')) { + warning( + false, + [ + 'Material-UI: the modal content node does not accept focus.', + 'For the benefit of assistive technologies, ' + + 'the tabIndex of the node is being set to "-1".', + ].join('\n'), + ); + rootRef.current.setAttribute('tabIndex', -1); + } + + rootRef.current.focus(); + } + + const enforceFocus = () => { + if (disableEnforceFocus || !isEnabled() || ignoreNextEnforceFocus.current) { + ignoreNextEnforceFocus.current = false; + return; + } + + if (!rootRef.current.contains(doc.activeElement)) { + rootRef.current.focus(); + } + }; + + const loopFocus = event => { + if (disableEnforceFocus || !isEnabled() || event.key !== 'Tab') { + return; + } + + // Make sure the next tab starts from the right place. + if (doc.activeElement === rootRef.current) { + // We need to ignore the next enforceFocus as + // it will try to move the focus back to the rootRef element. + ignoreNextEnforceFocus.current = true; + if (event.shiftKey) { + sentinelEnd.current.focus(); + } else { + sentinelStart.current.focus(); + } + } + }; + + doc.addEventListener('focus', enforceFocus, true); + doc.addEventListener('keydown', loopFocus, true); + + return () => { + doc.removeEventListener('focus', enforceFocus, true); + doc.removeEventListener('keydown', loopFocus, true); + + // restoreLastFocus() + if (!disableRestoreFocus) { + // Not all elements in IE 11 have a focus method. + // Because IE 11 market share is low, we accept the restore focus being broken + // and we silent the issue. + if (lastFocus.current.focus) { + lastFocus.current.focus(); + } + + lastFocus.current = null; + } + }; + }, [open]); + + return ( + +
+ {props.children} +
+ + ); +} + +TrapFocus.propTypes = { + /** + * A single child content element. + */ + children: PropTypes.element.isRequired, + /** + * If `true`, the modal will not automatically shift focus to itself when it opens, and + * replace it to the last focused element when it closes. + * This also works correctly with any modal children that have the `disableAutoFocus` prop. + * + * Generally this should never be set to `true` as it makes the modal less + * accessible to assistive technologies, like screen readers. + */ + disableAutoFocus: PropTypes.bool, + /** + * If `true`, the modal will not prevent focus from leaving the modal while open. + * + * Generally this should never be set to `true` as it makes the modal less + * accessible to assistive technologies, like screen readers. + */ + disableEnforceFocus: PropTypes.bool, + /** + * If `true`, the modal will not restore focus to previously focused element once + * modal is hidden. + */ + disableRestoreFocus: PropTypes.bool, + /** + * Do we still want to enforce the focus? + * This property helps nesting TrapFocus elements. + */ + isEnabled: PropTypes.func.isRequired, + /** + * If `true`, the modal is open. + */ + open: PropTypes.bool.isRequired, +}; + +TrapFocus.defaultProps = { + disableAutoFocus: false, + disableEnforceFocus: false, + disableRestoreFocus: false, +}; + +export default TrapFocus;