Modal close refactor

docs.md

@@ -584,18 +584,18 @@ A component passed using `previewComponent` will receive the following props:
 
 ### Parameters
 
--   `input` **[Object][156]** A `redux-form` [input][157] object
--   `meta` **[Object][156]** A `redux-form` [meta][158] object
--   `readFiles` **[Function][150]** A callback that is fired with new files and is expected to return an array of file objects with the `url` key set to the "read" value. This can be either a data URL or the public URL from a 3rd party API (optional, default `readFilesAsDataUrls`)
--   `multiple` **[Boolean][151]** A flag indicating whether or not to accept multiple files (optional, default `false`)
--   `accept` **[String][149]?** Value that defines the file types the file input should accept (e.g., ".doc,.docx"). More info: [https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#accept][166]
--   `capture` **(`"user"` \| `"environment"`)?** Value that specifies which camera to use, if the accept attribute indicates the input type of image or video. This is not available for all devices (e.g., desktops). More info: [https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#capture][167]
--   `onRemove` **[Function][150]** A callback fired when a file is removed (optional, default `noop`)
--   `previewComponent` **[Function][150]** A custom component that is used to display a preview of each attached file (optional, default `RenderPreview`)
--   `removeComponent` **[Function][150]** A custom component that receives `value` and `onRemove` props (optional, default `RemoveButton`)
--   `thumbnail` **[String][149]?** A placeholder image to display before the file is loaded
--   `hidePreview` **[Boolean][151]** A flag indicating whether or not to hide the file preview (optional, default `false`)
--   `selectText` **[String][149]?** An override for customizing the text that is displayed on the input's label. Defaults to 'Select File' or 'Select File(s)' depending on the `multiple` prop value
+-   `input` **[Object][153]** A `redux-form` [input][154] object
+-   `meta` **[Object][153]** A `redux-form` [meta][155] object
+-   `readFiles` **[Function][147]** A callback that is fired with new files and is expected to return an array of file objects with the `url` key set to the "read" value. This can be either a data URL or the public URL from a 3rd party API (optional, default `readFilesAsDataUrls`)
+-   `multiple` **[Boolean][148]** A flag indicating whether or not to accept multiple files (optional, default `false`)
+-   `accept` **[String][146]?** Value that defines the file types the file input should accept (e.g., ".doc,.docx"). More info: [https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#accept][163]
+-   `capture` **(`"user"` \| `"environment"`)?** Value that specifies which camera to use, if the accept attribute indicates the input type of image or video. This is not available for all devices (e.g., desktops). More info: [https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#capture][164]
+-   `onRemove` **[Function][147]** A callback fired when a file is removed (optional, default `noop`)
+-   `previewComponent` **[Function][147]** A custom component that is used to display a preview of each attached file (optional, default `RenderPreview`)
+-   `removeComponent` **[Function][147]** A custom component that receives `value` and `onRemove` props (optional, default `RemoveButton`)
+-   `thumbnail` **[String][146]?** A placeholder image to display before the file is loaded
+-   `hidePreview` **[Boolean][148]** A flag indicating whether or not to hide the file preview (optional, default `false`)
+-   `selectText` **[String][146]?** An override for customizing the text that is displayed on the input's label. Defaults to 'Select File' or 'Select File(s)' depending on the `multiple` prop value
 
 ### Examples
 
@@ -1739,7 +1739,8 @@ Note: this component requires custom styles. These styles can be imported from t
 ### Parameters
 
 -   `onClose` **[Function][147]** A handler for closing the modal. May be triggered via the close button, and outside click, or a key press.
--   `hideCloseButton` **[Boolean][148]?** A flag for hiding the default close button.
+-   `isOpen` **[Boolean][148]** A flag for showing the modal. (optional, default `true`)
+-   `preventClose` **[Boolean][148]** A flag for preventing the modal from being closed (close button, escape, or overlay click). (optional, default `false`)
 
 ### Examples
 

migration-guides/v6.0.0.md

@@ -12,6 +12,7 @@ This version contains the following breaking changes:
 9. The `previewComponent` for a file input no longer receives a `value` prop and `file` is a file object (with the url)
 10. The tag on `<Spinner />` now uses the class `spinner` in place of an id and supports additional classes
 11. `<TabBar />` now expects both `options` and `value` as required props.
+12. `<Modal />` no longer accepts the `hideCloseButton` prop
 
 The required changes for each item are detailed below.
 
@@ -238,6 +239,7 @@ Replace any styling rules that target `#spinner` with `.spinner`.
   }
 }
 ```
+
 ## 11. `<TabBar />` now expects both `options` and `value` as required props.
 
 Make sure that any instances of `<TabBar />` in your application are already sending these two props.
@@ -248,3 +250,23 @@ Make sure that any instances of `<TabBar />` in your application are already sen
     value={currentTab}
   />
 ```
+
+## 12. `<Modal />` no longer accepts the `hideCloseButton` prop.
+Replace `hideCloseButton` with `preventClose`. If you still need the modal to close on escape and/or by clicking the overlay, you can manually set those props. By default, `shouldCloseOnEsc` and `shouldCloseOnOverlayClick` will be set to the opposite of `preventClose` (default `false`).
+
+```jsx
+// Before
+<Modal hideCloseButton>
+  Modal content
+</Modal>
+
+// After
+<Modal preventClose>
+  Modal content
+</Modal>
+
+// With prop override
+<Modal preventClose shouldCloseOnEsc={true}>
+  Modal content
+</Modal>
+```

src/modal.js

@@ -13,7 +13,8 @@ import { isServer } from './utils'
  * @name Modal
  * @type Function
  * @param {Function} onClose - A handler for closing the modal. May be triggered via the close button, and outside click, or a key press.
- * @param {Boolean} [hideCloseButton] - A flag for hiding the default close button.
+ * @param {Boolean} [isOpen=true] - A flag for showing the modal.
+ * @param {Boolean} [preventClose=false] - A flag for preventing the modal from being closed (close button, escape, or overlay click).
  *
  * @example
  *
@@ -40,12 +41,14 @@ import { isServer } from './utils'
 
 const propTypes = {
   onClose: PropTypes.func.isRequired,
-  hideCloseButton: PropTypes.bool,
+  isOpen: PropTypes.bool,
+  preventClose: PropTypes.bool,
   children: PropTypes.node,
 }
 
 const defaultProps = {
-  hideCloseButton: false,
+  isOpen: true,
+  preventClose: false,
 }
 
 function getRootElement() {
@@ -57,30 +60,32 @@ function getRootElement() {
 
 // A wrapper for react-modal that adds some styles and a close button.
 // See https://github.com/reactjs/react-modal for usage.
-function Modal({ onClose, hideCloseButton, children, ...rest }) {
+function Modal({ isOpen, onClose, preventClose, children, ...rest }) {
+  const canClose = !preventClose
   return (
     <ReactModal
-      isOpen
+      isOpen={isOpen}
       onRequestClose={onClose}
       portalClassName="modal"
       className="modal-inner"
       overlayClassName="modal-fade-screen"
       bodyOpenClassName="modal-open"
       appElement={getRootElement()}
       ariaHideApp={isServer()} // Opt out of setting appElement on the server.
+      shouldCloseOnEsc={canClose}
+      shouldCloseOnOverlayClick={canClose}
       {...rest}
     >
       <div className="modal-content">{children}</div>
-      {!!onClose && !hideCloseButton && (
-        <>
-          <button
-            onClick={onClose}
-            className="modal-close"
-            aria-label="Close Modal"
-          >
-            ×
-          </button>
-        </>
+      {canClose && (
+        <button
+          type="button"
+          onClick={onClose}
+          className="modal-close"
+          aria-label="Close Modal"
+        >
+          ×
+      </button>
       )}
     </ReactModal>
   )

stories/modal.story.js

@@ -16,12 +16,12 @@ storiesOf('Modal', module)
       </div>
     )
   })
-  .add('without close button', () => {
+  .add('with close prevented', () => {
     const [modalShown, setModalShown] = useState(true)
     return (
       <div>
         {modalShown && (
-          <Modal onClose={() => setModalShown(false)} hideCloseButton>
+          <Modal onClose={() => setModalShown(false)} preventClose={true}>
             This is the modal content!
           </Modal>
         )}

test/modal.test.js

@@ -3,24 +3,51 @@ import { mount } from 'enzyme'
 import { Modal } from '../src/'
 import { noop } from 'lodash'
 
-test('Modal is shown by default', () => {
-  const wrapper = mount(<Modal onClose={noop} />)
-  expect(wrapper.find('.modal-content').exists()).toEqual(true)
-})
+describe('Modal', () => {
+  test('is shown by default', () => {
+    const wrapper = mount(<Modal onClose={noop} />)
+    expect(wrapper.find('.modal-content').exists()).toEqual(true)
+  })
+
+  test('can be hidden/animated by manually passing isOpen', () => {
+    const wrapper = mount(<Modal isOpen={false} onClose={noop} />)
+    expect(wrapper.find('.modal-content').exists()).toEqual(false)
+  })
+
+  test('calls close handler when close button is clicked', () => {
+    const onClose = jest.fn()
+    const wrapper = mount(<Modal onClose={onClose} />)
+    wrapper.find('.modal-close').simulate('click')
+    expect(onClose).toHaveBeenCalled()
+  })
 
-test('Modal can be hidden/animated by manually passing isOpen', () => {
-  const wrapper = mount(<Modal isOpen={false} onClose={noop} />)
-  expect(wrapper.find('.modal-content').exists()).toEqual(false)
-})
+  describe('when preventClose=true', () => {
+    test('hides close button', () => {
+      const wrapper = mount(<Modal preventClose={true} onClose={noop} />)
+      expect(wrapper.find('.modal-close').exists()).toEqual(false)
+    })
 
-test('Modal calls close handler when close button is clicked', () => {
-  const onClose = jest.fn()
-  const wrapper = mount(<Modal onClose={onClose} />)
-  wrapper.find('.modal-close').simulate('click')
-  expect(onClose).toHaveBeenCalled()
-})
+    test('does not close by escape key', () => {
+      const onClose = jest.fn()
+      const wrapper = mount(<Modal preventClose={true} onClose={onClose} />)
+      wrapper.find('.modal-content').simulate('keydown', { keyCode: 27 })
+      expect(onClose).not.toHaveBeenCalled()
+    })
+
+    test('does not close by overlay click', () => {
+      const onClose = jest.fn()
+      const overlayClass = 'test-overlay'
+      const wrapper = mount(<Modal preventClose={true} onClose={onClose} overlayClassName={overlayClass} />)
+      wrapper.find('.' + overlayClass).simulate('click')
+      expect(onClose).not.toHaveBeenCalled()
+      expect(wrapper.find('.modal-content').exists()).toEqual(true)
+    })
 
-test('Modal hides close button when hideCloseButton=true', () => {
-  const wrapper = mount(<Modal hideCloseButton onClose={noop} />)
-  expect(wrapper.find('.modal-close').exists()).toEqual(false)
+    test('allows individual prop overrides', () => {
+      const onClose = jest.fn()
+      const wrapper = mount(<Modal preventClose={true} onClose={onClose} shouldCloseOnEsc={true} />)
+      wrapper.find('.modal-content').simulate('keydown', { keyCode: 27 })
+      expect(onClose).toHaveBeenCalled()
+    })
+  })
 })