Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not overwrite default modal classes #554

Merged
merged 5 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion migration-guides/v7.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ This version contains the following breaking changes:
12. `<Modal />` no longer accepts the `hideCloseButton` prop
13. `<FlashMessageContainer />` component invokes `onDismiss` with redux-flash message object and allows message-specific overrides
14. `<Button />` and `<SubmitButton />` now accept a forwarded ref
15. `<Modal />` no longer overwrites the default modal or overlay class

The required changes for each item are detailed below.

Expand Down Expand Up @@ -331,4 +332,39 @@ function MyExample() {
<Button ref={ref}>Click me!</Button>
)
}
```
```

## 15. `<Modal />` no longer overwrites the default modal or overlay class
Prior to v7, passing in a custom class name to the modal would override the default class, which would often strip away all of the styling. Now, that class will be _added_ to the default class without the user having to include that class as well.

### Before
```jsx
// Before
<Modal className="custom-modal">
```
```scss
.custom-modal {
// copy over all styles from .modal-inner
// add additional custom styles
}
```
**or**
```jsx
// Before
<Modal className="modal-inner custom-modal">
```
```scss
.custom-modal {
// add additional custom styles
}
```
### After
```jsx
<Modal className="custom-modal">
```
```scss
.custom-modal {
// add additional custom styles
}
```
This logic applies to both the `className` and `overlayClassName` props, whether the use passes in a string or an object.
27 changes: 22 additions & 5 deletions src/modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react'
import PropTypes from 'prop-types'
import ReactModal from 'react-modal'
import { isServer } from './utils'
import classnames from 'classnames'

/**
* A modal component with a built-in close button. Uses [`react-modal`](https://github.com/reactjs/react-modal) under the hood, and can accept any props `react-modal` does.
Expand Down Expand Up @@ -39,11 +40,19 @@ import { isServer } from './utils'
* }
*/

const classNameObject = PropTypes.shape({
base: PropTypes.string.isRequired,
afterOpen: PropTypes.string.isRequired,
beforeClose: PropTypes.string.isRequired,
})

const propTypes = {
onClose: PropTypes.func.isRequired,
children: PropTypes.node,
className: PropTypes.oneOfType([PropTypes.string, classNameObject]),
isOpen: PropTypes.bool,
onClose: PropTypes.func.isRequired,
overlayClassName: PropTypes.oneOfType([PropTypes.string, classNameObject]),
preventClose: PropTypes.bool,
children: PropTypes.node,
}

const defaultProps = {
Expand All @@ -58,17 +67,25 @@ function getRootElement() {
return window.document.querySelector('body')
}

function wrapClassName(base, additional) {
if (typeof additional === 'object') {
return { ...additional, base: classnames(base, additional.base) }
}

return classnames(base, additional)
}

// A wrapper for react-modal that adds some styles and a close button.
// See https://github.com/reactjs/react-modal for usage.
function Modal({ isOpen, onClose, preventClose, children, ...rest }) {
function Modal({ isOpen, onClose, preventClose, children, className, overlayClassName, ...rest }) {
const canClose = !preventClose
return (
<ReactModal
isOpen={isOpen}
onRequestClose={onClose}
portalClassName="modal"
className="modal-inner"
overlayClassName="modal-fade-screen"
className={wrapClassName("modal-inner", className)}
overlayClassName={wrapClassName("modal-fade-screen", overlayClassName)}
bodyOpenClassName="modal-open"
appElement={getRootElement()}
ariaHideApp={isServer()} // Opt out of setting appElement on the server.
Expand Down
16 changes: 16 additions & 0 deletions stories/modal.story.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,19 @@ storiesOf('Modal', module)
</div>
)
})
.add('with custom class name configuration', () => {
const [modalShown, setModalShown] = useState(true)
return (
<div>
{modalShown && (
<Modal
onClose={() => setModalShown(false)}
className={{ base: 'custom', afterOpen: 'custom--after-open', beforeClose: '' }}
>
This is the modal content!
</Modal>
)}
<button onClick={() => setModalShown(true)}>Show modal</button>
</div>
)
})
31 changes: 31 additions & 0 deletions test/modal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,19 @@ import React from 'react'
import { mount } from 'enzyme'
import { Modal } from '../src/'
import { noop } from 'lodash'
import ReactModal from 'react-modal'

describe('Modal', () => {
// requestAnimationFrame is async, so the callback fails to trigger
// https://github.com/reactjs/react-modal/issues/903
beforeEach(() => {
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(cb => cb());
});

afterEach(() => {
window.requestAnimationFrame.mockRestore();
});

test('is shown by default', () => {
const wrapper = mount(<Modal onClose={noop} />)
expect(wrapper.find('.modal-content').exists()).toEqual(true)
Expand All @@ -21,6 +32,26 @@ describe('Modal', () => {
expect(onClose).toHaveBeenCalled()
})

test('adds additional class string to default class', () => {
const wrapper = mount(<Modal onClose={noop} className="custom" />)
expect(wrapper.find('.modal-inner.custom').exists()).toEqual(true)
})

test('adds additional overlay class string to default overlay class', () => {
const wrapper = mount(<Modal onClose={noop} overlayClassName="custom-overlay" />)
expect(wrapper.find('.modal-fade-screen.custom-overlay').exists()).toEqual(true)
})

test('adds additional class object to default class', () => {
const wrapper = mount(<Modal isOpen={true} onClose={noop} className={{ base: 'custom', afterOpen: 'modal-is-open', beforeClose: 'modal-before-close' }} />)
expect(wrapper.find('.modal-inner.custom.modal-is-open').exists()).toEqual(true)
})

test('adds additional overlay class object to default overlay class', () => {
const wrapper = mount(<Modal isOpen={true} onClose={noop} overlayClassName={{ base: 'custom', afterOpen: 'modal-is-open', beforeClose: 'modal-before-close' }} />)
expect(wrapper.find('.modal-fade-screen.custom.modal-is-open').exists()).toEqual(true)
})

describe('when preventClose=true', () => {
test('hides close button', () => {
const wrapper = mount(<Modal preventClose={true} onClose={noop} />)
Expand Down