From 1baebf41169dc7af273c44b8e348d3f17daff958 Mon Sep 17 00:00:00 2001 From: Andrew Fuller Date: Mon, 26 Jun 2017 19:50:05 -0500 Subject: [PATCH] [change] Track open body className appropriately --- docs/README.md | 2 +- docs/styles/classes.md | 4 ++++ specs/Modal.spec.js | 19 +++++++++++++++ specs/helper.js | 7 +++--- src/components/Modal.js | 39 ++++++------------------------ src/components/ModalPortal.js | 43 ++++++++++++++++++++++++++++++++++ src/helpers/ariaAppHider.js | 5 ---- src/helpers/refCount.js | 26 +++++++++++--------- src/helpers/safeHTMLElement.js | 7 ++++++ 9 files changed, 100 insertions(+), 52 deletions(-) create mode 100644 src/helpers/safeHTMLElement.js diff --git a/docs/README.md b/docs/README.md index da9f7e50..84c1d891 100644 --- a/docs/README.md +++ b/docs/README.md @@ -53,7 +53,7 @@ import ReactModal from 'react-modal'; */ className="ReactModal__Content" /* - String className to be applied to the document.body. + String className to be applied to the document.body (must be a constant string). See the `Styles` section for more details. */ bodyOpenClassName="ReactModal__Body--open" diff --git a/docs/styles/classes.md b/docs/styles/classes.md index 058a789a..57283c79 100644 --- a/docs/styles/classes.md +++ b/docs/styles/classes.md @@ -2,5 +2,9 @@ Sometimes it may be preferable to use CSS classes rather than inline styles. You can use the `className` and `overlayClassName` props to specify a given CSS class for each of those. You can override the default class that is added to `document.body` when the modal is open by defining a property `bodyOpenClassName`. + +It's required that `bodyOpenClassName` must be `constant string`, otherwise we would end up with a complex system to manage which class name +should appear or be removed from `document.body` from which modal (if using multiple modals simultaneously). + Note: If you provide those props all default styles will not be applied, leaving all styles under control of the CSS class. The `portalClassName` can also be used however there are no styles by default applied diff --git a/specs/Modal.spec.js b/specs/Modal.spec.js index c0bcd77f..867ba41b 100644 --- a/specs/Modal.spec.js +++ b/specs/Modal.spec.js @@ -239,6 +239,25 @@ describe('State', () => { unmountModal(); expect(!isBodyWithReactModalOpenClass()).toBeTruthy(); }); + + it('should not add classes to document.body for unopened modals', () => { + renderModal({ isOpen: true }); + expect(isBodyWithReactModalOpenClass()).toBeTruthy(); + renderModal({ isOpen: false, bodyOpenClassName: 'testBodyClass' }); + expect(!isBodyWithReactModalOpenClass('testBodyClass')).toBeTruthy() + }); + + it('should not remove classes from document.body when rendering unopened modal', () => { + renderModal({ isOpen: true }); + expect(isBodyWithReactModalOpenClass()).toBeTruthy(); + renderModal({ isOpen: false, bodyOpenClassName: 'testBodyClass' }); + renderModal({ isOpen: false }); + expect(!isBodyWithReactModalOpenClass('testBodyClass')).toBeTruthy() + expect(isBodyWithReactModalOpenClass()).toBeTruthy(); + renderModal({ isOpen: false }); + renderModal({ isOpen: false }); + expect(isBodyWithReactModalOpenClass()).toBeTruthy(); + }); it('adding/removing aria-hidden without an appElement will try to fallback to document.body', () => { ariaAppHider.documentNotReadyOrSSRTesting(); diff --git a/specs/helper.js b/specs/helper.js index 00c3fac5..04efd1c2 100644 --- a/specs/helper.js +++ b/specs/helper.js @@ -1,6 +1,6 @@ import React from 'react'; import ReactDOM from 'react-dom'; -import Modal from '../src/components/Modal'; +import Modal, { bodyOpenClassName } from '../src/components/Modal'; import TestUtils from 'react-dom/test-utils'; const divStack = []; @@ -19,8 +19,8 @@ if (!(String.prototype.hasOwnProperty('includes'))) { * open class. * @return {Boolean} */ -export const isBodyWithReactModalOpenClass = () => - document.body.className.includes('ReactModal__Body--open'); +export const isBodyWithReactModalOpenClass = (bodyClass = bodyOpenClassName) => + document.body.className.includes(bodyClass); /** * Returns a rendered dom element by class. @@ -109,6 +109,7 @@ export const renderModal = function(props, children, callback) { const currentDiv = document.createElement('div'); divStack.push(currentDiv); document.body.appendChild(currentDiv); + return ReactDOM.render( {children} , currentDiv, callback); diff --git a/src/components/Modal.js b/src/components/Modal.js index 55ff91e7..9f8b1eee 100644 --- a/src/components/Modal.js +++ b/src/components/Modal.js @@ -1,16 +1,14 @@ import React, { Component } from 'react'; import ReactDOM from 'react-dom'; import PropTypes from 'prop-types'; -import ExecutionEnvironment from 'exenv'; -import elementClass from 'element-class'; import ModalPortal from './ModalPortal'; import * as ariaAppHider from '../helpers/ariaAppHider'; -import * as refCount from '../helpers/refCount'; +import SafeHTMLElement from '../helpers/safeHTMLElement'; -const EE = ExecutionEnvironment; -const renderSubtreeIntoContainer = ReactDOM.unstable_renderSubtreeIntoContainer; +export const portalClassName = 'ReactModalPortal'; +export const bodyOpenClassName = 'ReactModal__Body--open'; -const SafeHTMLElement = EE.canUseDOM ? window.HTMLElement : {}; +const renderSubtreeIntoContainer = ReactDOM.unstable_renderSubtreeIntoContainer; function getParentElement(parentSelector) { return parentSelector(); @@ -62,8 +60,8 @@ export default class Modal extends Component { static defaultProps = { isOpen: false, - portalClassName: 'ReactModalPortal', - bodyOpenClassName: 'ReactModal__Body--open', + portalClassName, + bodyOpenClassName, ariaHideApp: true, closeTimeoutMS: 0, shouldCloseOnOverlayClick: true, @@ -99,10 +97,9 @@ export default class Modal extends Component { this.node = document.createElement('div'); this.node.className = this.props.portalClassName; - if (this.props.isOpen) refCount.add(this); - const parent = getParentElement(this.props.parentSelector); parent.appendChild(this.node); + this.renderPortal(this.props); } @@ -111,8 +108,6 @@ export default class Modal extends Component { // Stop unnecessary renders if modal is remaining closed if (!this.props.isOpen && !isOpen) return; - if (isOpen) refCount.add(this); - if (!isOpen) refCount.remove(this); const currentParent = getParentElement(this.props.parentSelector); const newParent = getParentElement(newProps.parentSelector); @@ -133,12 +128,6 @@ export default class Modal extends Component { componentWillUnmount() { if (!this.node) return; - refCount.remove(this); - - if (this.props.ariaHideApp) { - ariaAppHider.show(this.props.appElement); - } - const state = this.portal.state; const now = Date.now(); const closesAt = state.isOpen && this.props.closeTimeoutMS @@ -160,23 +149,9 @@ export default class Modal extends Component { ReactDOM.unmountComponentAtNode(this.node); const parent = getParentElement(this.props.parentSelector); parent.removeChild(this.node); - - if (refCount.count() === 0) { - elementClass(document.body).remove(this.props.bodyOpenClassName); - } } renderPortal = props => { - if (props.isOpen || refCount.count() > 0) { - elementClass(document.body).add(this.props.bodyOpenClassName); - } else { - elementClass(document.body).remove(this.props.bodyOpenClassName); - } - - if (props.ariaHideApp) { - ariaAppHider.toggle(props.isOpen, props.appElement); - } - this.portal = renderSubtreeIntoContainer(this, ( ), this.node); diff --git a/src/components/ModalPortal.js b/src/components/ModalPortal.js index 5c857203..066317d2 100644 --- a/src/components/ModalPortal.js +++ b/src/components/ModalPortal.js @@ -1,7 +1,11 @@ import React, { Component } from 'react'; import { PropTypes } from 'prop-types'; +import elementClass from 'element-class'; import * as focusManager from '../helpers/focusManager'; import scopeTab from '../helpers/scopeTab'; +import * as ariaAppHider from '../helpers/ariaAppHider'; +import * as refCount from '../helpers/refCount'; +import SafeHTMLElement from '../helpers/safeHTMLElement'; // so that our CSS is statically analyzable const CLASS_NAMES = { @@ -38,6 +42,9 @@ export default class ModalPortal extends Component { PropTypes.string, PropTypes.object ]), + bodyOpenClassName: PropTypes.string, + ariaHideApp: PropTypes.bool, + appElement: PropTypes.instanceOf(SafeHTMLElement), onAfterOpen: PropTypes.func, onRequestClose: PropTypes.func, closeTimeoutMS: PropTypes.number, @@ -67,6 +74,15 @@ export default class ModalPortal extends Component { } componentWillReceiveProps(newProps) { + if (process.env.NODE_ENV !== "production") { + if (newProps.bodyOpenClassName !== this.props.bodyOpenClassName) { + // eslint-disable-next-line no-console + console.warn( + 'React-Modal: "bodyOpenClassName" prop has been modified. ' + + 'This may cause unexpected behavior when multiple modals are open.' + ); + } + } // Focus only needs to be set once when the modal is being opened if (!this.props.isOpen && newProps.isOpen) { this.setFocusAfterRender(true); @@ -84,6 +100,7 @@ export default class ModalPortal extends Component { } componentWillUnmount() { + this.beforeClose(); clearTimeout(this.closeTimer); } @@ -99,12 +116,37 @@ export default class ModalPortal extends Component { this.content = content; } + beforeOpen() { + const { appElement, ariaHideApp, bodyOpenClassName } = this.props; + refCount.add(bodyOpenClassName); + // Add body class + elementClass(document.body).add(bodyOpenClassName); + // Add aria-hidden to appElement + if (ariaHideApp) { + ariaAppHider.hide(appElement); + } + } + + beforeClose() { + const { appElement, ariaHideApp, bodyOpenClassName } = this.props; + refCount.remove(bodyOpenClassName); + // Remove class if no more modals are open + if (refCount.count(bodyOpenClassName) === 0) { + elementClass(document.body).remove(bodyOpenClassName); + } + // Reset aria-hidden attribute if all modals have been removed + if (ariaHideApp && refCount.totalCount() < 1) { + ariaAppHider.show(appElement); + } + } + afterClose = () => { focusManager.returnFocus(); focusManager.teardownScopedFocus(); } open = () => { + this.beforeOpen(); if (this.state.afterOpen && this.state.beforeClose) { clearTimeout(this.closeTimer); this.setState({ beforeClose: false }); @@ -122,6 +164,7 @@ export default class ModalPortal extends Component { } close = () => { + this.beforeClose(); if (this.props.closeTimeoutMS > 0) { this.closeWithTimeout(); } else { diff --git a/src/helpers/ariaAppHider.js b/src/helpers/ariaAppHider.js index c9c7d1d9..20b85abf 100644 --- a/src/helpers/ariaAppHider.js +++ b/src/helpers/ariaAppHider.js @@ -48,11 +48,6 @@ export function show(appElement) { (appElement || globalElement).removeAttribute('aria-hidden'); } -export function toggle(shouldHide, appElement) { - const apply = shouldHide ? hide : show; - apply(appElement); -} - export function documentNotReadyOrSSRTesting() { globalElement = null; } diff --git a/src/helpers/refCount.js b/src/helpers/refCount.js index 5066dfa7..ca5c6752 100644 --- a/src/helpers/refCount.js +++ b/src/helpers/refCount.js @@ -1,19 +1,23 @@ -const modals = []; +const modals = {}; -export function add(element) { - if (modals.indexOf(element) === -1) { - modals.push(element); +export function add(bodyClass) { + // Set variable and default if none + if (!modals[bodyClass]) { + modals[bodyClass] = 0; } + modals[bodyClass] += 1; } -export function remove(element) { - const index = modals.indexOf(element); - if (index === -1) { - return; +export function remove(bodyClass) { + if (modals[bodyClass]) { + modals[bodyClass] -= 1; } - modals.splice(index, 1); } -export function count() { - return modals.length; +export function count(bodyClass) { + return modals[bodyClass]; +} + +export function totalCount() { + return Object.keys(modals).reduce((acc, curr) => acc + modals[curr], 0); } diff --git a/src/helpers/safeHTMLElement.js b/src/helpers/safeHTMLElement.js new file mode 100644 index 00000000..7a8752b0 --- /dev/null +++ b/src/helpers/safeHTMLElement.js @@ -0,0 +1,7 @@ +import ExecutionEnvironment from 'exenv'; + +const EE = ExecutionEnvironment; + +const SafeHTMLElement = EE.canUseDOM ? window.HTMLElement : {}; + +export default SafeHTMLElement;