LaunchPadLab / lp-components

Our Components
http://lp-components.herokuapp.com
MIT License
5 stars 1 forks source link

Allow Modal's onClose to be null and derive other props from it #517

Closed chawes13 closed 2 years ago

chawes13 commented 2 years ago

Instead of having to pass in a function and several related props, derive the props from the lack of having a close function supplied. It's common for hideCloseButton to be the only prop that is considered, which can lead to defects with the user still being able to close a modal via the keyboard or pressing the overlay.

// Current State
<Modal
  onClose={() => setModalShown(false)}
  hideCloseButton={!canClose}
  shouldCloseOnEsc={canClose}
  shouldCloseOnOverlayClick={canClose}
/>

// Proposed
<Modal onClose={canClose ? () => setModalShown(false) : null} />
// Proposed implementation
function Modal({ onClose, children, ...rest }) {
    return (
    <ReactModal
      isOpen
      onRequestClose={onClose ?? noop}
      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={!!onClose}
      shouldCloseOnOverlayClick={!!onClose}
      {...rest}
    >
      <div className="modal-content">{children}</div>
      {!!onClose  && (
        <>
          <button
            onClick={onClose}
            className="modal-close"
            aria-label="Close Modal"
          >
            ×
          </button>
        </>
      )}
    </ReactModal>
  )
}
chawes13 commented 2 years ago

On second thought, I don't really love that UX. What if we encapsulated the broader concept of preventing close, i.e., preventClose?

// Proposed implementation
function Modal({ onClose, preventClose=false, children, ...rest }) {
    return (
    <ReactModal
      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={!preventClose}
      shouldCloseOnOverlayClick={!preventClose}
      {...rest}
    >
      <div className="modal-content">{children}</div>
      {!preventClose  && (
        <>
          <button
            onClick={onClose}
            className="modal-close"
            aria-label="Close Modal"
          >
            ×
          </button>
        </>
      )}
    </ReactModal>
  )
}