Skip to content

Commit

Permalink
fix: Presence of has-body when (un)mounting several modals (#1262)
Browse files Browse the repository at this point in the history
fix: Presence of has-body when (un)mounting several modals
  • Loading branch information
ptbrowne authored Nov 25, 2019
2 parents 96f27f1 + 76c5993 commit 046769b
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 3 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"jest": "yarn test:jest",
"screenshots": "node scripts/screenshots.js --screenshot-dir ./screenshots --styleguide-dir ./build/react",
"test": "yarn test:jest",
"test:jest": "jest --verbose",
"test:jest": "env USE_REACT=true jest --verbose",
"transpile": "env BABEL_ENV=transpilation babel react/ --out-dir transpiled/react --verbose && babel helpers/ --out-dir transpiled/helpers --verbose",
"posttranspile": "postcss transpiled/react/stylesheet.css --replace",
"travis-deploy-once": "travis-deploy-once",
Expand Down
16 changes: 14 additions & 2 deletions react/Modal/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ const ModalTitle = props => {
return <ModalHeader {...props} />
}

// Present on the body when a Modal is shown, changes the
// z-index of alerts so they appear in front of the modal
export const BODY_CLASS = 'has-modal'

class Modal extends Component {
constructor(props) {
super(props)
Expand All @@ -38,11 +42,19 @@ class Modal extends Component {
'If your modal has not label you should add an invisible one with `aria-label` props for a11y purposes.'
)
}
document.body.classList.add('has-modal')

const hasBodyClass = document.body.classList.contains(BODY_CLASS)
if (!hasBodyClass) {
document.body.classList.add(BODY_CLASS)
this.shouldRemoveBodyClass = true
}
}

componentWillUnmount() {
document.body.classList.remove('has-modal')
// Do not remove the class if it was not added by us
if (this.shouldRemoveBodyClass) {
document.body.classList.remove(BODY_CLASS)
}
}

render() {
Expand Down
73 changes: 73 additions & 0 deletions react/Modal/index.spec.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import React from 'react'
import { act } from 'react-dom/test-utils'
import { useState } from 'react'
import { mount } from 'enzyme'
import Modal, { BODY_CLASS } from './index'

describe('Modal', () => {
const Example = () => {
const [showModal, setShowModal] = useState(false)
const [showModal2, setShowModal2] = useState(false)
return (
<div>
{showModal ? <Modal title="example 1">1</Modal> : null}
{showModal2 ? <Modal title="example 2">2</Modal> : null}
<button onClick={() => setShowModal(true)} />
<button onClick={() => setShowModal2(true)} />
<button onClick={() => setShowModal2(false)} />
<button onClick={() => setShowModal(false)} />
</div>
)
}

const clickNthButton = (root, n) =>
root
.find('button')
.at(n)
.props()
.onClick()

const mountFirstModal = root => clickNthButton(root, 0)
const mountSecondModal = root => clickNthButton(root, 1)
const unmountSecondModal = root => clickNthButton(root, 2)
const unmountFirstModal = root => clickNthButton(root, 3)

const hasModalBodyClass = () => document.body.classList.contains(BODY_CLASS)

afterEach(() => {
document.body.classList.remove(BODY_CLASS)
})

it('should apply a class to the body when mounted', () => {
const root = mount(<Example />)
expect(hasModalBodyClass()).toBe(false)
act(() => {
mountFirstModal(root)
})
expect(hasModalBodyClass()).toBe(true)
act(() => {
unmountFirstModal(root)
})
expect(hasModalBodyClass()).toBe(false)
})

it('should not remove the class if two modals were mounted and the second one is removed', () => {
const root = mount(<Example />)
expect(hasModalBodyClass()).toBe(false)
act(() => {
mountFirstModal(root)
})
expect(hasModalBodyClass()).toBe(true)
act(() => {
mountSecondModal(root)
})
act(() => {
unmountSecondModal(root)
})
expect(hasModalBodyClass()).toBe(true)
act(() => {
unmountFirstModal(root)
})
expect(hasModalBodyClass()).toBe(false)
})
})

0 comments on commit 046769b

Please sign in to comment.