From d6ea3062110b9daaec3da40be8f3a12e6c49e432 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Fri, 2 Aug 2019 12:34:23 +0200 Subject: [PATCH] Add disableScrollLock prop --- docs/pages/api/modal.md | 1 + .../src/pages/components/modal/ServerModal.js | 51 ++++++++++++++++++ .../pages/components/modal/ServerModal.tsx | 53 +++++++++++++++++++ .../src/pages/components/modal/SimpleModal.js | 3 +- .../pages/components/modal/SimpleModal.tsx | 3 +- docs/src/pages/components/modal/modal.md | 15 ++++-- packages/material-ui/src/Modal/Modal.d.ts | 1 + packages/material-ui/src/Modal/Modal.js | 30 +++++------ .../material-ui/src/Modal/ModalManager.js | 46 ++++++++-------- .../src/Modal/ModalManager.test.js | 18 +++---- packages/material-ui/src/Portal/Portal.d.ts | 2 +- 11 files changed, 166 insertions(+), 57 deletions(-) create mode 100644 docs/src/pages/components/modal/ServerModal.js create mode 100644 docs/src/pages/components/modal/ServerModal.tsx diff --git a/docs/pages/api/modal.md b/docs/pages/api/modal.md index 4a30d7e831b59b..1e99c56f64667d 100644 --- a/docs/pages/api/modal.md +++ b/docs/pages/api/modal.md @@ -39,6 +39,7 @@ This component shares many concepts with [react-overlays](https://react-bootstra | disableEscapeKeyDown | bool | false | If `true`, hitting escape will not fire any callback. | | disablePortal | bool | false | Disable the portal behavior. The children stay within it's parent DOM hierarchy. | | disableRestoreFocus | bool | false | If `true`, the modal will not restore focus to previously focused element once modal is hidden. | +| disableScrollLock | bool | false | Disable the scroll lock behavior. | | hideBackdrop | bool | false | If `true`, the backdrop is not rendered. | | keepMounted | bool | false | Always keep the children in the DOM. This prop can be useful in SEO situation or when you want to maximize the responsiveness of the Modal. | | onBackdropClick | func | | Callback fired when the backdrop is clicked. | diff --git a/docs/src/pages/components/modal/ServerModal.js b/docs/src/pages/components/modal/ServerModal.js new file mode 100644 index 00000000000000..c1e36518c63fcd --- /dev/null +++ b/docs/src/pages/components/modal/ServerModal.js @@ -0,0 +1,51 @@ +import React from 'react'; +import { makeStyles } from '@material-ui/core/styles'; +import Modal from '@material-ui/core/Modal'; + +const useStyles = makeStyles(theme => ({ + root: { + transform: 'translateZ(0)', + height: 300, + flexGrow: 1, + }, + modal: { + display: 'flex', + padding: theme.spacing(1), + alignItems: 'center', + justifyContent: 'center', + }, + paper: { + width: 400, + backgroundColor: theme.palette.background.paper, + border: '2px solid #000', + boxShadow: theme.shadows[5], + padding: theme.spacing(2, 4, 4), + }, +})); + +export default function ServerModal() { + const classes = useStyles(); + const rootRef = React.useRef(null); + + return ( +
+ rootRef.current} + > +
+

Server-side modal

+

+ You can disable the JavaScript, you will still see me. +

+
+ +
+ ); +} diff --git a/docs/src/pages/components/modal/ServerModal.tsx b/docs/src/pages/components/modal/ServerModal.tsx new file mode 100644 index 00000000000000..e21272ed0e7e36 --- /dev/null +++ b/docs/src/pages/components/modal/ServerModal.tsx @@ -0,0 +1,53 @@ +import React from 'react'; +import { makeStyles, Theme, createStyles } from '@material-ui/core/styles'; +import Modal from '@material-ui/core/Modal'; + +const useStyles = makeStyles((theme: Theme) => + createStyles({ + root: { + transform: 'translateZ(0)', + height: 300, + flexGrow: 1, + }, + modal: { + display: 'flex', + padding: theme.spacing(1), + alignItems: 'center', + justifyContent: 'center', + }, + paper: { + width: 400, + backgroundColor: theme.palette.background.paper, + border: '2px solid #000', + boxShadow: theme.shadows[5], + padding: theme.spacing(2, 4, 4), + }, + }), +); + +export default function ServerModal() { + const classes = useStyles(); + const rootRef = React.useRef(null); + + return ( +
+ rootRef.current} + > +
+

Server-side modal

+

+ You can disable the JavaScript, you will still see me. +

+
+ +
+ ); +} diff --git a/docs/src/pages/components/modal/SimpleModal.js b/docs/src/pages/components/modal/SimpleModal.js index a0777b8beeef9f..7b84ab2236fcc3 100644 --- a/docs/src/pages/components/modal/SimpleModal.js +++ b/docs/src/pages/components/modal/SimpleModal.js @@ -25,7 +25,6 @@ const useStyles = makeStyles(theme => ({ border: '2px solid #000', boxShadow: theme.shadows[5], padding: theme.spacing(2, 4, 4), - outline: 'none', }, })); @@ -56,7 +55,7 @@ export default function SimpleModal() { onClose={handleClose} >
- +

Text in a modal

Duis mollis, est non commodo luctus, nisi erat porttitor ligula.

diff --git a/docs/src/pages/components/modal/SimpleModal.tsx b/docs/src/pages/components/modal/SimpleModal.tsx index a529e8f52409e6..80f585b0a3424e 100644 --- a/docs/src/pages/components/modal/SimpleModal.tsx +++ b/docs/src/pages/components/modal/SimpleModal.tsx @@ -26,7 +26,6 @@ const useStyles = makeStyles((theme: Theme) => border: '2px solid #000', boxShadow: theme.shadows[5], padding: theme.spacing(2, 4, 4), - outline: 'none', }, }), ); @@ -58,7 +57,7 @@ export default function SimpleModal() { onClose={handleClose} >
- +

Text in a modal

Duis mollis, est non commodo luctus, nisi erat porttitor ligula.

diff --git a/docs/src/pages/components/modal/modal.md b/docs/src/pages/components/modal/modal.md index a7f14acea5813a..e6779cf5cebcf9 100644 --- a/docs/src/pages/components/modal/modal.md +++ b/docs/src/pages/components/modal/modal.md @@ -34,6 +34,8 @@ Modal is a lower-level construct that is leveraged by the following components: {{"demo": "pages/components/modal/SimpleModal.js"}} +Notice that you can disable the blue outline with the `outline: 0` CSS property. + ## Performance The content of the modal is **lazily mounted** into the DOM. @@ -85,16 +87,23 @@ Additionally, you may give a description of your modal with the `aria-describedb ```jsx -

+

``` - 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. + +## Server-side modal + +React [doesn't support](https://github.com/facebook/react/issues/13097) the [`createPortal()`](https://reactjs.org/docs/portals.html) API on the server. +In order to make it work, you need to disable this feature with the `disablePortal` prop: + +{{"demo": "pages/components/modal/ServerModal.js"}} diff --git a/packages/material-ui/src/Modal/Modal.d.ts b/packages/material-ui/src/Modal/Modal.d.ts index a907deebd50694..d4496339665747 100644 --- a/packages/material-ui/src/Modal/Modal.d.ts +++ b/packages/material-ui/src/Modal/Modal.d.ts @@ -16,6 +16,7 @@ export interface ModalProps disableEscapeKeyDown?: boolean; disablePortal?: PortalProps['disablePortal']; disableRestoreFocus?: boolean; + disableScrollLock?: boolean; hideBackdrop?: boolean; keepMounted?: boolean; manager?: ModalManager; diff --git a/packages/material-ui/src/Modal/Modal.js b/packages/material-ui/src/Modal/Modal.js index a8638c4899da05..6c72ad1c976686 100644 --- a/packages/material-ui/src/Modal/Modal.js +++ b/packages/material-ui/src/Modal/Modal.js @@ -22,15 +22,10 @@ function getHasTransition(props) { return props.children ? props.children.props.hasOwnProperty('in') : false; } +// A modal manager used to track and manage the state of open Modals. // Modals don't open on the server so this won't conflict with concurrent requests. const defaultManager = new ModalManager(); -function getModal(modal, mountNodeRef, modalRef) { - modal.current.modalRef = modalRef.current; - modal.current.mountNode = mountNodeRef.current; - return modal.current; -} - export const styles = theme => ({ /* Styles applied to the root element. */ root: { @@ -73,6 +68,7 @@ const Modal = React.forwardRef(function Modal(props, ref) { disableEscapeKeyDown = false, disablePortal = false, disableRestoreFocus = false, + disableScrollLock = false, hideBackdrop = false, keepMounted = false, manager = defaultManager, @@ -93,9 +89,14 @@ const Modal = React.forwardRef(function Modal(props, ref) { const hasTransition = getHasTransition(props); const getDoc = () => ownerDocument(mountNodeRef.current); + const getModal = () => { + modal.current.modalRef = modalRef.current; + modal.current.mountNode = mountNodeRef.current; + return modal.current; + }; const handleMounted = () => { - manager.mount(getModal(modal, mountNodeRef, modalRef)); + manager.mount(getModal(), { disableScrollLock }); // Fix a bug on Chrome where the scroll isn't initially 0. modalRef.current.scrollTop = 0; @@ -104,7 +105,7 @@ const Modal = React.forwardRef(function Modal(props, ref) { const handleOpen = useEventCallback(() => { const resolvedContainer = getContainer(container) || getDoc().body; - manager.add(getModal(modal, mountNodeRef, modalRef), resolvedContainer); + manager.add(getModal(), resolvedContainer); // The element was already mounted. if (modalRef.current) { @@ -112,10 +113,7 @@ const Modal = React.forwardRef(function Modal(props, ref) { } }); - const isTopModal = React.useCallback( - () => manager.isTopModal(getModal(modal, mountNodeRef, modalRef)), - [manager], - ); + const isTopModal = React.useCallback(() => manager.isTopModal(getModal()), [manager]); const handlePortalRef = useEventCallback(node => { mountNodeRef.current = node; @@ -136,7 +134,7 @@ const Modal = React.forwardRef(function Modal(props, ref) { }); const handleClose = React.useCallback(() => { - manager.remove(getModal(modal, mountNodeRef, modalRef)); + manager.remove(getModal()); }, [manager]); React.useEffect(() => { @@ -316,6 +314,10 @@ Modal.propTypes = { * modal is hidden. */ disableRestoreFocus: PropTypes.bool, + /** + * Disable the scroll lock behavior. + */ + disableScrollLock: PropTypes.bool, /** * If `true`, the backdrop is not rendered. */ @@ -328,8 +330,6 @@ Modal.propTypes = { keepMounted: PropTypes.bool, /** * @ignore - * - * A modal manager used to track and manage the state of open Modals. */ manager: PropTypes.object, /** diff --git a/packages/material-ui/src/Modal/ModalManager.js b/packages/material-ui/src/Modal/ModalManager.js index f8351b8dc1df6e..ba0e1066438ae8 100644 --- a/packages/material-ui/src/Modal/ModalManager.js +++ b/packages/material-ui/src/Modal/ModalManager.js @@ -58,32 +58,30 @@ function findIndexOf(containerInfo, callback) { return idx; } -function handleNewContainer(containerInfo) { - // We are only interested in the actual `style` here because we will override it. - const restoreStyle = { - overflow: containerInfo.container.style.overflow, - 'padding-right': containerInfo.container.style.paddingRight, - }; - - const style = { - overflow: 'hidden', - }; - +function handleContainer(containerInfo, props) { + const restoreStyle = {}; + const style = {}; const restorePaddings = []; let fixedNodes; - if (containerInfo.overflowing) { - const scrollbarSize = getScrollbarSize(); + if (!props.disableScrollLock) { + restoreStyle.overflow = containerInfo.container.style.overflow; + restoreStyle['padding-right'] = containerInfo.container.style.paddingRight; + style.overflow = 'hidden'; - // Use computed style, here to get the real padding to add our scrollbar width. - style['padding-right'] = `${getPaddingRight(containerInfo.container) + scrollbarSize}px`; + if (isOverflowing(containerInfo.container)) { + const scrollbarSize = getScrollbarSize(); - // .mui-fixed is a global helper. - fixedNodes = ownerDocument(containerInfo.container).querySelectorAll('.mui-fixed'); - [].forEach.call(fixedNodes, node => { - restorePaddings.push(node.style.paddingRight); - node.style.paddingRight = `${getPaddingRight(node) + scrollbarSize}px`; - }); + // Use computed style, here to get the real padding to add our scrollbar width. + style['padding-right'] = `${getPaddingRight(containerInfo.container) + scrollbarSize}px`; + + // .mui-fixed is a global helper. + fixedNodes = ownerDocument(containerInfo.container).querySelectorAll('.mui-fixed'); + [].forEach.call(fixedNodes, node => { + restorePaddings.push(node.style.paddingRight); + node.style.paddingRight = `${getPaddingRight(node) + scrollbarSize}px`; + }); + } } Object.keys(style).forEach(key => { @@ -137,7 +135,6 @@ export default class ModalManager { // this.contaniners[containerIndex] = { // modals: [], // container, - // overflowing, // restore: null, // } this.contaniners = []; @@ -169,7 +166,6 @@ export default class ModalManager { this.contaniners.push({ modals: [modal], container, - overflowing: isOverflowing(container), restore: null, hiddenSiblingNodes, }); @@ -177,12 +173,12 @@ export default class ModalManager { return modalIndex; } - mount(modal) { + mount(modal, props) { const containerIndex = findIndexOf(this.contaniners, item => item.modals.indexOf(modal) !== -1); const containerInfo = this.contaniners[containerIndex]; if (!containerInfo.restore) { - containerInfo.restore = handleNewContainer(containerInfo); + containerInfo.restore = handleContainer(containerInfo, props); } } diff --git a/packages/material-ui/src/Modal/ModalManager.test.js b/packages/material-ui/src/Modal/ModalManager.test.js index 3a693781cd10af..9d3ef4915fde9f 100644 --- a/packages/material-ui/src/Modal/ModalManager.test.js +++ b/packages/material-ui/src/Modal/ModalManager.test.js @@ -29,7 +29,7 @@ describe('ModalManager', () => { const modal = {}; const modalManager2 = new ModalManager(); const idx = modalManager2.add(modal, container1); - modalManager2.mount(modal); + modalManager2.mount(modal, {}); assert.strictEqual(modalManager2.add(modal, container1), idx); modalManager2.remove(modal); }); @@ -47,7 +47,7 @@ describe('ModalManager', () => { it('should add modal1', () => { const idx = modalManager.add(modal1, container1); - modalManager.mount(modal1); + modalManager.mount(modal1, {}); assert.strictEqual(idx, 0, 'should be the first modal'); assert.strictEqual(modalManager.isTopModal(modal1), true); }); @@ -71,7 +71,7 @@ describe('ModalManager', () => { it('should add modal2 2', () => { const idx = modalManager.add(modal2, container1); - modalManager.mount(modal2); + modalManager.mount(modal2, {}); assert.strictEqual(idx, 2, 'should be the "third" modal'); assert.strictEqual(modalManager.isTopModal(modal2), true); assert.strictEqual( @@ -125,7 +125,7 @@ describe('ModalManager', () => { const modal = {}; modalManager.add(modal, container1); - modalManager.mount(modal); + modalManager.mount(modal, {}); assert.strictEqual(container1.style.overflow, 'hidden'); assert.strictEqual(container1.style.paddingRight, `${20 + getScrollbarSize()}px`); assert.strictEqual(fixedNode.style.paddingRight, `${14 + getScrollbarSize()}px`); @@ -138,7 +138,7 @@ describe('ModalManager', () => { it('should restore styles correctly if none existed before', () => { const modal = {}; modalManager.add(modal, container1); - modalManager.mount(modal); + modalManager.mount(modal, {}); assert.strictEqual(container1.style.overflow, 'hidden'); assert.strictEqual(container1.style.paddingRight, `${20 + getScrollbarSize()}px`); assert.strictEqual(fixedNode.style.paddingRight, `${0 + getScrollbarSize()}px`); @@ -168,11 +168,11 @@ describe('ModalManager', () => { const modal1 = {}; const modal2 = {}; modalManager.add(modal1, container3); - modalManager.mount(modal1); + modalManager.mount(modal1, {}); expect(container3.children[0]).to.be.ariaHidden; modalManager.add(modal2, container4); - modalManager.mount(modal2); + modalManager.mount(modal2, {}); expect(container4.children[0]).to.be.ariaHidden; modalManager.remove(modal2); @@ -242,7 +242,7 @@ describe('ModalManager', () => { const modal = { modalRef: container2.children[0] }; modalManager.add(modal, container2); - modalManager.mount(modal); + modalManager.mount(modal, {}); expect(container2.children[0]).not.to.be.ariaHidden; modalManager.remove(modal, container2); expect(container2.children[0]).to.be.ariaHidden; @@ -259,7 +259,7 @@ describe('ModalManager', () => { container2.appendChild(sibling2); modalManager.add(modal, container2); - modalManager.mount(modal); + modalManager.mount(modal, {}); expect(container2.children[0]).not.to.be.ariaHidden; modalManager.remove(modal, container2); expect(container2.children[0]).to.be.ariaHidden; diff --git a/packages/material-ui/src/Portal/Portal.d.ts b/packages/material-ui/src/Portal/Portal.d.ts index 9bf46654b63e86..31d373b1070014 100644 --- a/packages/material-ui/src/Portal/Portal.d.ts +++ b/packages/material-ui/src/Portal/Portal.d.ts @@ -3,7 +3,7 @@ import { PortalProps } from '../Portal'; export interface PortalProps { children: React.ReactElement; - container?: React.ReactInstance | (() => React.ReactInstance) | null; + container?: React.ReactInstance | (() => React.ReactInstance | null) | null; disablePortal?: boolean; onRendered?: () => void; }