diff --git a/package.json b/package.json index 57b30570e..b7c4c19c0 100644 --- a/package.json +++ b/package.json @@ -94,7 +94,8 @@ "lodash.tonumber": "^4.0.3", "prop-types": "^15.5.8", "react-popper": "^0.7.2", - "react-transition-group": "^2.2.0" + "react-portal": "^4.1.2", + "react-transition-group": "^2.2.1" }, "peerDependencies": { "react": "^0.14.9 || ^15.3.0 || ^16.0.0", @@ -117,7 +118,7 @@ "css-loader": "^0.25.0", "ejs": "^2.5.1", "enzyme": "^3.0.0", - "enzyme-adapter-react-16": "^1.0.0", + "enzyme-adapter-react-16": "^1.1.1", "eslint": "^4.1.1", "eslint-config-airbnb": "^15.1.0", "eslint-plugin-import": "^2.6.0", diff --git a/src/Modal.js b/src/Modal.js index 086106203..d23c2247a 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -1,6 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import ReactDOM from 'react-dom'; +import { Portal } from 'react-portal'; import classNames from 'classnames'; import Fade from './Fade'; import { @@ -73,40 +73,62 @@ class Modal extends React.Component { constructor(props) { super(props); - this.originalBodyPadding = null; - this.isBodyOverflowing = false; - this.togglePortal = this.togglePortal.bind(this); + this._element = null; + this._originalBodyPadding = null; this.handleBackdropClick = this.handleBackdropClick.bind(this); this.handleEscape = this.handleEscape.bind(this); - this.destroy = this.destroy.bind(this); this.onOpened = this.onOpened.bind(this); this.onClosed = this.onClosed.bind(this); + + this.state = { + isOpen: props.isOpen, + }; + + if (props.isOpen) { + this.init(); + } } componentDidMount() { - if (this.props.isOpen) { - this.togglePortal(); - } if (this.props.onEnter) { this.props.onEnter(); } + + if (this.state.isOpen && this.props.autoFocus) { + this.setFocus(); + } + + this._isMounted = true; } - componentDidUpdate(prevProps) { - if (this.props.isOpen !== prevProps.isOpen) { - // handle portal events/dom updates - this.togglePortal(); - } else if (this._element) { - // rerender portal - this.renderIntoSubtree(); + componentWillReceiveProps(nextProps) { + if (nextProps.isOpen && !this.props.isOpen) { + this.setState({ isOpen: nextProps.isOpen }); + } + } + + componentWillUpdate(nextProps, nextState) { + if (nextState.isOpen && !this.state.isOpen) { + this.init(); + } + } + + componentDidUpdate(prevProps, prevState) { + if (this.props.autoFocus && this.state.isOpen && !prevState.isOpen) { + this.setFocus(); } } componentWillUnmount() { - this.destroy(); if (this.props.onExit) { this.props.onExit(); } + + if (this.state.isOpen) { + this.destroy(); + } + + this._isMounted = false; } onOpened(node, isAppearing) { @@ -116,19 +138,24 @@ class Modal extends React.Component { onClosed(node) { // so all methods get called before it is unmounted - setTimeout(() => this.destroy(), 0); this.props.onClosed(); (this.props.modalTransition.onExited || noop)(node); + this.destroy(); + + if (this._isMounted) { + this.setState({ isOpen: false }); + } } - handleEscape(e) { - if (this.props.keyboard && e.keyCode === 27 && this.props.toggle) { - this.props.toggle(); + setFocus() { + if (this._dialog && this._dialog.parentNode && typeof this._dialog.parentNode.focus === 'function') { + this._dialog.parentNode.focus(); } } handleBackdropClick(e) { - if (this.props.backdrop !== true) return; + e.stopPropagation(); + if (!this.props.isOpen || this.props.backdrop !== true) return; const container = this._dialog; @@ -137,65 +164,39 @@ class Modal extends React.Component { } } - togglePortal() { - if (this.props.isOpen) { - if (this.props.autoFocus) { - this._focus = true; - } - this.show(); - } else { - this.hide(); - } - } - - destroy() { - if (this._element) { - ReactDOM.unmountComponentAtNode(this._element); - document.body.removeChild(this._element); - this._element = null; - } - - if (this.bodyClassAdded) { - const modalOpenClassName = mapToCssModules('modal-open', this.props.cssModule); - // Use regex to prevent matching `modal-open` as part of a different class, e.g. `my-modal-opened` - const modalOpenClassNameRegex = new RegExp(`(^| )${modalOpenClassName}( |$)`); - document.body.className = document.body.className.replace(modalOpenClassNameRegex, ' ').trim(); - this.bodyClassAdded = false; + handleEscape(e) { + if (this.props.isOpen && this.props.keyboard && e.keyCode === 27 && this.props.toggle) { + this.props.toggle(); } - setScrollbarWidth(this.originalBodyPadding); - } - - hide() { - this.renderIntoSubtree(); } - show() { - if (this._dialog) { - if (this.props.toggle) { - this.props.toggle(true); - } - return; - } - const classes = document.body.className; + init() { this._element = document.createElement('div'); this._element.setAttribute('tabindex', '-1'); this._element.style.position = 'relative'; this._element.style.zIndex = this.props.zIndex; - this.originalBodyPadding = getOriginalBodyPadding(); + this._originalBodyPadding = getOriginalBodyPadding(); conditionallyUpdateScrollbar(); document.body.appendChild(this._element); - if (!this.bodyClassAdded) { - document.body.className = classNames( - classes, - mapToCssModules('modal-open', this.props.cssModule) - ); - this.bodyClassAdded = true; - } + document.body.className = classNames( + document.body.className, + mapToCssModules('modal-open', this.props.cssModule) + ); + } - this.renderIntoSubtree(); + destroy() { + document.body.removeChild(this._element); + this._element = null; + + const modalOpenClassName = mapToCssModules('modal-open', this.props.cssModule); + // Use regex to prevent matching `modal-open` as part of a different class, e.g. `my-modal-opened` + const modalOpenClassNameRegex = new RegExp(`(^| )${modalOpenClassName}( |$)`); + document.body.className = document.body.className.replace(modalOpenClassNameRegex, ' ').trim(); + + setScrollbarWidth(this._originalBodyPadding); } renderModalDialog() { @@ -203,6 +204,7 @@ class Modal extends React.Component { return (
{ this._dialog = c; }} - {...attributes} >
+
+ + {this.renderModalDialog()} + + +
+ + ); } - } - - renderChildren() { - const { - wrapClassName, - modalClassName, - backdropClassName, - cssModule, - isOpen, - backdrop, - role, - labelledBy - } = this.props; - - const modalAttributes = { - onClickCapture: this.handleBackdropClick, - onKeyUp: this.handleEscape, - style: { display: 'block' }, - 'aria-labelledby': labelledBy, - role, - tabIndex: '-1' - }; - - const hasTransition = this.props.fade; - const modalTransition = { - ...Fade.defaultProps, - ...this.props.modalTransition, - baseClass: hasTransition ? this.props.modalTransition.baseClass : '', - timeout: hasTransition ? this.props.modalTransition.timeout : 0, - }; - const backdropTransition = { - ...Fade.defaultProps, - ...this.props.backdropTransition, - baseClass: hasTransition ? this.props.backdropTransition.baseClass : '', - timeout: hasTransition ? this.props.backdropTransition.timeout : 0, - }; - return ( -
- - {this.renderModalDialog()} - - -
- ); - } - render() { return null; } } diff --git a/src/__tests__/Modal.spec.js b/src/__tests__/Modal.spec.js index 7e2916f15..acd66ae96 100644 --- a/src/__tests__/Modal.spec.js +++ b/src/__tests__/Modal.spec.js @@ -25,6 +25,19 @@ describe('Modal', () => { jest.clearAllTimers(); }); + it('should render modal portal into DOM', () => { + isOpen = true; + const wrapper = mount( + + Yo! + + ); + + jest.runTimersToTime(300); + expect(wrapper.childAt(0).children().length).not.toBe(0); + wrapper.unmount(); + }); + it('should render with the class "modal-dialog"', () => { isOpen = true; const wrapper = mount( @@ -34,7 +47,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal-dialog').length).toBe(1); wrapper.unmount(); }); @@ -48,7 +60,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal-backdrop').length).toBe(1); wrapper.unmount(); }); @@ -62,7 +73,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal-backdrop').length).toBe(1); wrapper.unmount(); }); @@ -76,7 +86,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal-dialog').length).toBe(1); expect(document.getElementsByClassName('modal-backdrop').length).toBe(0); wrapper.unmount(); @@ -91,7 +100,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal-dialog').length).toBe(1); expect(document.getElementsByClassName('my-custom-modal').length).toBe(1); wrapper.unmount(); @@ -106,7 +114,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal-dialog').length).toBe(1); expect(document.getElementsByClassName('modal-dialog')[0].style.maxWidth).toBe('95%'); wrapper.unmount(); @@ -122,7 +129,6 @@ describe('Modal', () => { // Modal should appear instantaneously jest.runTimersToTime(1); - expect(wrapper.childAt(0).children().length).toBe(0); const matchedModals = document.getElementsByClassName('fadeless-modal'); const matchedModal = matchedModals[0]; @@ -149,7 +155,6 @@ describe('Modal', () => { ); jest.runTimersToTime(20); - expect(wrapper.childAt(0).children().length).toBe(0); const matchedModals = document.getElementsByClassName('custom-timeout-modal'); @@ -167,7 +172,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.querySelectorAll('.modal.my-custom-modal').length).toBe(1); wrapper.unmount(); }); @@ -181,7 +185,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('my-custom-modal').length).toBe(1); wrapper.unmount(); }); @@ -195,7 +198,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.querySelectorAll('.modal-content.my-custom-modal').length).toBe(1); wrapper.unmount(); }); @@ -209,7 +211,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.querySelectorAll('.modal-backdrop.my-custom-modal').length).toBe(1); wrapper.unmount(); }); @@ -223,7 +224,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal-dialog').length).toBe(1); expect(document.getElementsByClassName('modal-crazy').length).toBe(1); wrapper.unmount(); @@ -239,7 +239,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal').length).toBe(1); expect(document.getElementsByClassName('modal-backdrop').length).toBe(1); wrapper.unmount(); @@ -254,7 +253,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal')[0].getAttribute('role')).toBe('dialog'); wrapper.unmount(); }); @@ -268,7 +266,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal')[0].getAttribute('role')).toBe('alert'); wrapper.unmount(); }); @@ -282,7 +279,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal')[0].getAttribute('aria-labelledby')).toBe('myModalTitle'); wrapper.unmount(); }); @@ -310,6 +306,7 @@ describe('Modal', () => { jest.runTimersToTime(300); expect(isOpen).toBe(false); + expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal').length).toBe(0); expect(document.getElementsByClassName('modal-backdrop').length).toBe(0); @@ -404,8 +401,8 @@ describe('Modal', () => { wrapper.unmount(); }); - it('should not call togglePortal when isOpen does not change', () => { - jest.spyOn(Modal.prototype, 'togglePortal'); + it('should not call init when isOpen does not change', () => { + jest.spyOn(Modal.prototype, 'init'); jest.spyOn(Modal.prototype, 'componentDidUpdate'); const wrapper = mount( @@ -415,7 +412,7 @@ describe('Modal', () => { jest.runTimersToTime(300); expect(isOpen).toBe(false); - expect(Modal.prototype.togglePortal).not.toHaveBeenCalled(); + expect(Modal.prototype.init).not.toHaveBeenCalled(); expect(Modal.prototype.componentDidUpdate).not.toHaveBeenCalled(); wrapper.setProps({ @@ -424,39 +421,12 @@ describe('Modal', () => { jest.runTimersToTime(300); expect(isOpen).toBe(false); - expect(Modal.prototype.togglePortal).not.toHaveBeenCalled(); + expect(Modal.prototype.init).not.toHaveBeenCalled(); expect(Modal.prototype.componentDidUpdate).toHaveBeenCalled(); wrapper.unmount(); }); - it('should renderIntoSubtree when props updated', () => { - isOpen = true; - jest.spyOn(Modal.prototype, 'togglePortal'); - jest.spyOn(Modal.prototype, 'renderIntoSubtree'); - const wrapper = mount( - - Yo! - - ); - - jest.runTimersToTime(300); - expect(isOpen).toBe(true); - expect(Modal.prototype.togglePortal.mock.calls.length).toEqual(1); - expect(Modal.prototype.renderIntoSubtree.mock.calls.length).toEqual(1); - - wrapper.setProps({ - isOpen: isOpen - }); - jest.runTimersToTime(300); - - expect(isOpen).toBe(true); - expect(Modal.prototype.togglePortal.mock.calls.length).toEqual(1); - expect(Modal.prototype.renderIntoSubtree.mock.calls.length).toEqual(2); - - wrapper.unmount(); - }); - it('should close modal when escape key pressed', () => { isOpen = true; const wrapper = mount( @@ -591,14 +561,36 @@ describe('Modal', () => { jest.runTimersToTime(300); expect(instance._element).toBeTruthy(); - instance.destroy(); + toggle(); + wrapper.setProps({ + isOpen: isOpen + }); + jest.runTimersToTime(300); + expect(isOpen).toBe(false); expect(instance._element).toBe(null); - instance.destroy(); wrapper.unmount(); }); + it('should destroy this._element on unmount', () => { + isOpen = true; + const wrapper = mount( + + + + ); + const instance = wrapper.instance(); + + jest.runTimersToTime(300); + expect(instance._element).toBeTruthy(); + + wrapper.unmount(); + jest.runTimersToTime(300); + + expect(instance._element).toBe(null); + }); + it('should render nested modals', () => { isOpen = true; isOpenNested = true; @@ -613,7 +605,6 @@ describe('Modal', () => { ); jest.runTimersToTime(300); - expect(wrapper.childAt(0).children().length).toBe(0); expect(document.getElementsByClassName('modal-dialog').length).toBe(2); expect(document.body.className).toBe('modal-open modal-open'); diff --git a/yarn.lock b/yarn.lock index 6804cdbd6..a47c9975b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2740,24 +2740,25 @@ entities@^1.1.1, entities@~1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/entities/-/entities-1.1.1.tgz#6e5c2d0a5621b5dadaecef80b90edfb5cd7772f0" -enzyme-adapter-react-16@^1.0.0: - version "1.0.2" - resolved "https://registry.yarnpkg.com/enzyme-adapter-react-16/-/enzyme-adapter-react-16-1.0.2.tgz#8c6f431f17c69e1e9eeb25ca4bd92f31971eb2dd" +enzyme-adapter-react-16@^1.1.1: + version "1.1.1" + resolved "https://registry.yarnpkg.com/enzyme-adapter-react-16/-/enzyme-adapter-react-16-1.1.1.tgz#a8f4278b47e082fbca14f5bfb1ee50ee650717b4" dependencies: - enzyme-adapter-utils "^1.0.0" + enzyme-adapter-utils "^1.3.0" lodash "^4.17.4" object.assign "^4.0.4" object.values "^1.0.4" - prop-types "^15.5.10" + prop-types "^15.6.0" + react-reconciler "^0.7.0" react-test-renderer "^16.0.0-0" -enzyme-adapter-utils@^1.0.0: - version "1.0.1" - resolved "https://registry.yarnpkg.com/enzyme-adapter-utils/-/enzyme-adapter-utils-1.0.1.tgz#fcd81223339a55a312f7552641e045c404084009" +enzyme-adapter-utils@^1.3.0: + version "1.3.0" + resolved "https://registry.yarnpkg.com/enzyme-adapter-utils/-/enzyme-adapter-utils-1.3.0.tgz#d6c85756826c257a8544d362cc7a67e97ea698c7" dependencies: lodash "^4.17.4" object.assign "^4.0.4" - prop-types "^15.5.10" + prop-types "^15.6.0" enzyme@^3.0.0: version "3.1.0" @@ -2786,7 +2787,17 @@ error-ex@^1.2.0: dependencies: is-arrayish "^0.2.1" -es-abstract@^1.6.1, es-abstract@^1.7.0: +es-abstract@^1.6.1: + version "1.10.0" + resolved "https://registry.yarnpkg.com/es-abstract/-/es-abstract-1.10.0.tgz#1ecb36c197842a00d8ee4c2dfd8646bb97d60864" + dependencies: + es-to-primitive "^1.1.1" + function-bind "^1.1.1" + has "^1.0.1" + is-callable "^1.1.3" + is-regex "^1.0.4" + +es-abstract@^1.7.0: version "1.9.0" resolved "https://registry.yarnpkg.com/es-abstract/-/es-abstract-1.9.0.tgz#690829a07cae36b222e7fd9b75c0d0573eb25227" dependencies: @@ -3762,6 +3773,10 @@ has-flag@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/has-flag/-/has-flag-2.0.0.tgz#e8207af1cc7b30d446cc70b734b5e8be18f88d51" +has-symbols@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/has-symbols/-/has-symbols-1.0.0.tgz#ba1a8f1af2a0fc39650f5c850367704122063b44" + has-unicode@^2.0.0: version "2.0.1" resolved "https://registry.yarnpkg.com/has-unicode/-/has-unicode-2.0.1.tgz#e0e6fe6a28cf51138855e086d1691e771de2a8b9" @@ -5501,17 +5516,18 @@ object-is@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/object-is/-/object-is-1.0.1.tgz#0aa60ec9989a0b3ed795cf4d06f62cf1ad6539b6" -object-keys@^1.0.10, object-keys@^1.0.8: +object-keys@^1.0.11, object-keys@^1.0.8: version "1.0.11" resolved "https://registry.yarnpkg.com/object-keys/-/object-keys-1.0.11.tgz#c54601778ad560f1142ce0e01bcca8b56d13426d" object.assign@^4.0.4: - version "4.0.4" - resolved "https://registry.yarnpkg.com/object.assign/-/object.assign-4.0.4.tgz#b1c9cc044ef1b9fe63606fc141abbb32e14730cc" + version "4.1.0" + resolved "https://registry.yarnpkg.com/object.assign/-/object.assign-4.1.0.tgz#968bf1100d7956bb3ca086f006f846b3bc4008da" dependencies: define-properties "^1.1.2" - function-bind "^1.1.0" - object-keys "^1.0.10" + function-bind "^1.1.1" + has-symbols "^1.0.0" + object-keys "^1.0.11" object.entries@^1.0.4: version "1.0.4" @@ -6375,10 +6391,25 @@ react-popper@^0.7.2: popper.js "^1.12.5" prop-types "^15.5.10" +react-portal@^4.1.2: + version "4.1.2" + resolved "https://registry.yarnpkg.com/react-portal/-/react-portal-4.1.2.tgz#7f28f3c8c2ed5c541907c0ed0f24e8996acf627f" + dependencies: + prop-types "^15.5.8" + react-prism@^4.1.0: version "4.3.1" resolved "https://registry.yarnpkg.com/react-prism/-/react-prism-4.3.1.tgz#cf6ce5d77ee5d9cb91eed02801e3af7547d19214" +react-reconciler@^0.7.0: + version "0.7.0" + resolved "https://registry.yarnpkg.com/react-reconciler/-/react-reconciler-0.7.0.tgz#9614894103e5f138deeeb5eabaf3ee80eb1d026d" + dependencies: + fbjs "^0.8.16" + loose-envify "^1.1.0" + object-assign "^4.1.1" + prop-types "^15.6.0" + react-router@^3.2.0: version "3.2.0" resolved "https://registry.yarnpkg.com/react-router/-/react-router-3.2.0.tgz#62b6279d589b70b34e265113e4c0a9261a02ed36" @@ -6441,14 +6472,22 @@ react-side-effect@^1.1.0: exenv "^1.2.1" shallowequal "^1.0.1" -react-test-renderer@^16.0.0, react-test-renderer@^16.0.0-0: +react-test-renderer@^16.0.0: version "16.0.0" resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-16.0.0.tgz#9fe7b8308f2f71f29fc356d4102086f131c9cb15" dependencies: fbjs "^0.8.16" object-assign "^4.1.1" -react-transition-group@^2.2.0: +react-test-renderer@^16.0.0-0: + version "16.2.0" + resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-16.2.0.tgz#bddf259a6b8fcd8555f012afc8eacc238872a211" + dependencies: + fbjs "^0.8.16" + object-assign "^4.1.1" + prop-types "^15.6.0" + +react-transition-group@^2.2.1: version "2.2.1" resolved "https://registry.yarnpkg.com/react-transition-group/-/react-transition-group-2.2.1.tgz#e9fb677b79e6455fd391b03823afe84849df4a10" dependencies: