From c844ab1f0c4e02c2169e8ad19b18cc59bf4bd3c6 Mon Sep 17 00:00:00 2001 From: Hieu Lam Date: Wed, 22 May 2019 03:58:52 +0700 Subject: [PATCH] fix(Modal): handle init modal in SSR (#1495) * fix(Modal): handle init modal in SSR * fix(Modal): move init to did mount of component and rewrite test cases * fix(Modal): handle tigger to re-render in case `isOpen` is true on init --- src/Modal.js | 11 ++--- src/__tests__/Modal.spec.js | 92 ++++++++++++++++++++----------------- 2 files changed, 54 insertions(+), 49 deletions(-) diff --git a/src/Modal.js b/src/Modal.js index c698fbc84..e663a9dc9 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -100,15 +100,16 @@ class Modal extends React.Component { this.manageFocusAfterClose = this.manageFocusAfterClose.bind(this); this.state = { - isOpen: props.isOpen, + isOpen: false, }; + } - if (props.isOpen) { + componentDidMount() { + if (this.props.isOpen) { this.init(); + this.setState({ isOpen: true }) } - } - componentDidMount() { if (this.props.onEnter) { this.props.onEnter(); } @@ -341,7 +342,6 @@ class Modal extends React.Component { } = this.props; if (!!this._element && (this.state.isOpen || !unmountOnClose)) { - const isModalHidden = !!this._element && !this.state.isOpen && !unmountOnClose; this._element.style.display = isModalHidden ? 'none' : 'block'; @@ -415,7 +415,6 @@ class Modal extends React.Component { ); } - return null; } } diff --git a/src/__tests__/Modal.spec.js b/src/__tests__/Modal.spec.js index c1d039084..77b642aae 100644 --- a/src/__tests__/Modal.spec.js +++ b/src/__tests__/Modal.spec.js @@ -3,6 +3,12 @@ import React from 'react'; import { mount, shallow } from 'enzyme'; import { Modal, ModalBody, ModalHeader, ModalFooter, Button } from '../'; +const didMount = (component) => { + const wrapper = mount(component); + wrapper.setProps({ fakefield: 'fakeToUpdate' }); + return wrapper; +} + describe('Modal', () => { let isOpen; let toggle; @@ -28,7 +34,7 @@ describe('Modal', () => { it('should render modal portal into DOM', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -41,7 +47,7 @@ describe('Modal', () => { it('should render with the class "modal-dialog"', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -54,7 +60,7 @@ describe('Modal', () => { it('should render external content when present', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( ×}> Yo! @@ -68,7 +74,7 @@ describe('Modal', () => { it('should render with the backdrop with the class "modal-backdrop" by default', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -81,7 +87,7 @@ describe('Modal', () => { it('should render with the backdrop with the class "modal-backdrop" when backdrop is "static"', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -94,7 +100,7 @@ describe('Modal', () => { it('should not render with the backdrop with the class "modal-backdrop" when backdrop is "false"', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -108,7 +114,7 @@ describe('Modal', () => { it('should render with the class "modal-dialog-scrollable" when scrollable is "true"', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -121,7 +127,7 @@ describe('Modal', () => { it('should render with class "modal-dialog" and have custom class name if provided', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -135,7 +141,7 @@ describe('Modal', () => { it('should render with class "modal-dialog" w/o centered class if not provided', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -149,7 +155,7 @@ describe('Modal', () => { it('should render with class "modal-dialog" and centered class if provided', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -163,7 +169,7 @@ describe('Modal', () => { it('should render with additional props if provided', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -177,7 +183,7 @@ describe('Modal', () => { it('should render without fade transition if provided with fade={false}', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Howdy! @@ -198,7 +204,7 @@ describe('Modal', () => { it('should render when expected when passed modalTransition and backdropTransition props', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( { it('should render with class "modal" and have custom class name if provided with modalClassName', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -234,7 +240,7 @@ describe('Modal', () => { it('should render with custom class name if provided with wrapClassName', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -247,7 +253,7 @@ describe('Modal', () => { it('should render with class "modal-content" and have custom class name if provided with contentClassName', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -260,7 +266,7 @@ describe('Modal', () => { it('should render with class "modal-backdrop" and have custom class name if provided with backdropClassName', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -273,7 +279,7 @@ describe('Modal', () => { it('should render with the class "modal-${size}" when size is passed', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -288,7 +294,7 @@ describe('Modal', () => { it('should render modal when isOpen is true', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -302,7 +308,7 @@ describe('Modal', () => { it('should render modal with default role of "dialog"', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -315,7 +321,7 @@ describe('Modal', () => { it('should render modal with provided role', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -328,7 +334,7 @@ describe('Modal', () => { it('should render modal with aria-labelledby provided labelledBy', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -340,7 +346,7 @@ describe('Modal', () => { }); it('should not render modal when isOpen is false', () => { - const wrapper = mount( + const wrapper = didMount( Yo! @@ -354,7 +360,7 @@ describe('Modal', () => { }); it('should toggle modal', () => { - const wrapper = mount( + const wrapper = didMount( Yo! @@ -383,7 +389,7 @@ describe('Modal', () => { jest.spyOn(Modal.prototype, 'onClosed'); const onOpened = jest.fn(); const onClosed = jest.fn(); - const wrapper = mount( + const wrapper = didMount( Yo! @@ -424,7 +430,7 @@ describe('Modal', () => { it('should call onClosed & onOpened when fade={false}', () => { const onOpened = jest.fn(); const onClosed = jest.fn(); - const wrapper = mount( + const wrapper = didMount( Yo! @@ -485,7 +491,7 @@ describe('Modal', () => { it('should close modal when escape key pressed', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -528,7 +534,7 @@ describe('Modal', () => { it('should not close modal when escape key pressed when keyboard is false', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -563,7 +569,7 @@ describe('Modal', () => { it('should close modal when clicking backdrop', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( @@ -598,7 +604,7 @@ describe('Modal', () => { it('should not close modal when clicking backdrop and backdrop is "static"', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( @@ -624,7 +630,7 @@ describe('Modal', () => { it('should destroy this._element', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( @@ -648,7 +654,7 @@ describe('Modal', () => { it('should destroy this._element when unmountOnClose prop set to true', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( @@ -672,7 +678,7 @@ describe('Modal', () => { it('should not destroy this._element when unmountOnClose prop set to false', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( @@ -696,7 +702,7 @@ describe('Modal', () => { it('should destroy this._element on unmount', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( @@ -715,7 +721,7 @@ describe('Modal', () => { it('should render nested modals', () => { isOpen = true; isOpenNested = true; - const wrapper = mount( + const wrapper = didMount( @@ -744,7 +750,7 @@ describe('Modal', () => { // set a body class which includes modal-open document.body.className = 'my-modal-opened'; - const wrapper = mount( + const wrapper = didMount( Yo! @@ -786,7 +792,7 @@ describe('Modal', () => { it('should call onEnter & onExit props if provided', () => { const onEnter = jest.fn(); const onExit = jest.fn(); - const wrapper = mount( + const wrapper = didMount( Yo! @@ -837,7 +843,7 @@ describe('Modal', () => { it('should allow focus on only focusable elements', () => { isOpen = true; - const wrapper = mount( + const wrapper = didMount( Modal title @@ -876,7 +882,7 @@ describe('Modal', () => { const mock = jest.fn(); - const wrapper = mount( + const wrapper = didMount( @@ -902,7 +908,7 @@ describe('Modal', () => { ); - const wrapper = mount(); + const wrapper = didMount(); const button = wrapper .find('.focus') .hostNodes() @@ -925,7 +931,7 @@ describe('Modal', () => { ); - const wrapper = mount(); + const wrapper = didMount(); const button = wrapper .find('.focus') .hostNodes() @@ -948,7 +954,7 @@ describe('Modal', () => { ); - const wrapper = mount(); + const wrapper = didMount(); const button = wrapper .find('.focus') .hostNodes() @@ -971,7 +977,7 @@ describe('Modal', () => { ); - const wrapper = mount(); + const wrapper = didMount(); const button = wrapper .find('.focus') .hostNodes()