refactor Modal mechanism

2020-02-05 18:48:30 +08:00 · 2020-02-05 18:48:30 +08:00 · b083ee8788
commit b083ee8788
parent 54c4891fd2
6 changed files with 153 additions and 132 deletions
--- a/resources/assets/src/components/Modal.tsx
+++ b/resources/assets/src/components/Modal.tsx
@ -33,115 +33,131 @@ type Props = {
  footer?: React.ReactNode
  onConfirm?(payload: { value: string }): void
  onDismiss?(): void
+  onClose?(): void
 }

 export type ModalResult = {
  value: string
 }

-const Modal = React.forwardRef<HTMLDivElement, ModalOptions & Props>(
-  (props, forwardedRef) => {
-    const [hidden, setHidden] = useState(false)
-    const [value, setValue] = useState(props.input!)
-    const [valid, setValid] = useState(true)
-    const [validatorMessage, setValidatorMessage] = useState('')
-    const ref = (forwardedRef ??
-      useRef<HTMLDivElement>(null)) as React.RefObject<HTMLDivElement>
+const Modal: React.FC<ModalOptions & Props> = props => {
+  const [value, setValue] = useState(props.input!)
+  const [valid, setValid] = useState(true)
+  const [validatorMessage, setValidatorMessage] = useState('')
+  const ref = useRef<HTMLDivElement>(null)

-    const handleInputChange = (event: React.ChangeEvent<HTMLInputElement>) => {
-      setValue(event.target.value)
+  const handleInputChange = (event: React.ChangeEvent<HTMLInputElement>) => {
+    setValue(event.target.value)
+  }
+
+  const confirm = () => {
+    const { validator } = props
+    if (typeof validator === 'function') {
+      const result = validator(value)
+      if (typeof result === 'string') {
+        setValidatorMessage(result)
+        setValid(false)
+        return
+      }
    }

-    const confirm = () => {
-      const { validator } = props
-      if (typeof validator === 'function') {
-        const result = validator(value)
-        if (typeof result === 'string') {
-          setValidatorMessage(result)
-          setValid(false)
-          return
-        }
-      }
+    props.onConfirm?.({ value })
+    $(ref.current!).modal('hide')

-      setHidden(true)
-      props.onConfirm?.({ value })
-      $(ref.current!).modal('hide')
+    // The "hidden.bs.modal" event can't be trigged automatically when testing.
+    /* istanbul ignore next */
+    if (process.env.NODE_ENV === 'test') {
+      $(ref.current!).trigger('hidden.bs.modal')
+    }
+  }
+
+  const dismiss = () => {
+    props.onDismiss?.()
+    $(ref.current!).modal('hide')
+
+    /* istanbul ignore next */
+    if (process.env.NODE_ENV === 'test') {
+      $(ref.current!).trigger('hidden.bs.modal')
+    }
+  }
+
+  useEffect(() => {
+    if (!props.show) {
+      return
    }

-    const dismiss = () => {
-      setHidden(true)
-      props.onDismiss?.()
+    const onHidden = () => props.onClose?.()
+
+    const el = $(ref.current!)
+    el.on('hidden.bs.modal', onHidden)
+
+    return () => {
+      el.off('hidden.bs.modal', onHidden)
    }
+  }, [props.onClose, props.show])

-    useEffect(() => {
-      const onHide = () => {
-        if (!hidden) {
-          dismiss()
-        }
-      }
-      const onHidden = () => setHidden(false)
+  useEffect(() => {
+    if (props.show) {
+      setTimeout(() => $(ref.current!).modal('show'), 50)
+    }
+  }, [props.show])

-      const el = $(ref.current!)
-      el.on('hide.bs.modal', onHide).on('hidden.bs.modal', onHidden)
+  if (!props.show) {
+    return null
+  }

-      return () => {
-        el.off('hide.bs.modal', onHide).off('hidden.bs.modal', onHidden)
-      }
-    }, [hidden, props.onDismiss])
-
-    return (
+  return (
+    <div
+      id={props.id}
+      className="modal fade"
+      tabIndex={-1}
+      role="dialog"
+      aria-hidden={!props.show}
+      ref={ref}
+    >
      <div
-        id={props.id}
-        className={`modal fade ${props.show ? 'show' : ''}`}
-        tabIndex={-1}
-        role="dialog"
-        aria-hidden={!props.show}
-        ref={ref}
+        className={`modal-dialog ${
+          props.center ? 'modal-dialog-centered' : ''
+        }`}
+        role="document"
      >
-        <div
-          className={`modal-dialog ${
-            props.center ? 'modal-dialog-centered' : ''
-          }`}
-          role="document"
-        >
-          <div className={`modal-content bg-${props.type}`}>
-            <ModalHeader
-              show={props.showHeader}
-              title={props.title}
-              onDismiss={dismiss}
-            />
-            <ModalBody
-              text={props.text}
-              dangerousHTML={props.dangerousHTML}
-              showInput={props.mode === 'prompt'}
-              value={value}
-              choices={props.choices}
-              onChange={handleInputChange}
-              inputType={props.inputType}
-              placeholder={props.placeholder}
-              invalid={!valid}
-              validatorMessage={validatorMessage}
-            >
-              {props.children}
-            </ModalBody>
-            <ModalFooter
-              showCancelButton={props.mode !== 'alert'}
-              flexFooter={props.flexFooter}
-              okButtonType={props.okButtonType}
-              okButtonText={props.okButtonText}
-              cancelButtonType={props.cancelButtonType}
-              cancelButtonText={props.cancelButtonText}
-              onConfirm={confirm}
-              onDismiss={dismiss}
-            >
-              {props.footer}
-            </ModalFooter>
-          </div>
+        <div className={`modal-content bg-${props.type}`}>
+          <ModalHeader
+            show={props.showHeader}
+            title={props.title}
+            onDismiss={dismiss}
+          />
+          <ModalBody
+            text={props.text}
+            dangerousHTML={props.dangerousHTML}
+            showInput={props.mode === 'prompt'}
+            value={value}
+            choices={props.choices}
+            onChange={handleInputChange}
+            inputType={props.inputType}
+            placeholder={props.placeholder}
+            invalid={!valid}
+            validatorMessage={validatorMessage}
+          >
+            {props.children}
+          </ModalBody>
+          <ModalFooter
+            showCancelButton={props.mode !== 'alert'}
+            flexFooter={props.flexFooter}
+            okButtonType={props.okButtonType}
+            okButtonText={props.okButtonText}
+            cancelButtonType={props.cancelButtonType}
+            cancelButtonText={props.cancelButtonText}
+            onConfirm={confirm}
+            onDismiss={dismiss}
+          >
+            {props.footer}
+          </ModalFooter>
        </div>
      </div>
-    )
-  },
-)
+    </div>
+  )
+}

 Modal.displayName = 'Modal'

--- a/resources/assets/src/scripts/modal.tsx
+++ b/resources/assets/src/scripts/modal.tsx
@ -1,4 +1,3 @@
-import $ from 'jquery'
 import React from 'react'
 import ReactDOM from 'react-dom'
 import Modal, { ModalOptions, ModalResult } from '../components/Modal'
@ -8,25 +7,21 @@ export function showModal(options: ModalOptions = {}): Promise<ModalResult> {
    const container = document.createElement('div')
    document.body.appendChild(container)

-    const ref = React.createRef<HTMLDivElement>()
+    const handleClose = () => {
+      ReactDOM.unmountComponentAtNode(container)
+      document.body.removeChild(container)
+    }
+
    ReactDOM.render(
      <Modal
        {...options}
-        ref={ref}
+        show
        center
        onConfirm={resolve}
        onDismiss={reject}
+        onClose={handleClose}
      />,
      container,
    )
-
-    $(ref.current!)
-      .modal('show')
-      .on('hidden.bs.modal', () => {
-        setTimeout(() => {
-          ReactDOM.unmountComponentAtNode(container)
-          container.remove()
-        }, 0)
-      })
  })
 }
--- a/resources/assets/src/views/user/OAuth/ModalCreate.tsx
+++ b/resources/assets/src/views/user/OAuth/ModalCreate.tsx
@ -3,7 +3,9 @@ import Modal from '../../../components/Modal'
 import { trans } from '../../../scripts/i18n'

 interface Props {
+  show: boolean
  onCreate(name: string, redirect: string): Promise<void>
+  onClose(): void
 }

 const ModalCreate: React.FC<Props> = props => {
@ -30,8 +32,10 @@ const ModalCreate: React.FC<Props> = props => {
  return (
    <Modal
      id="modal-create"
+      show={props.show}
      onConfirm={handleComplete}
      onDismiss={handleDismiss}
+      onClose={props.onClose}
    >
      <table className="table">
        <tbody>
--- a/resources/assets/src/views/user/OAuth/index.tsx
+++ b/resources/assets/src/views/user/OAuth/index.tsx
@ -15,6 +15,7 @@ type Exception = {
 const OAuth: React.FC = () => {
  const [apps, setApps] = useState<App[]>([])
  const [isLoading, setIsLoading] = useState(false)
+  const [showModalCreate, setShowModalCreate] = useState(false)

  useEffect(() => {
    const getApps = async () => {
@ -26,6 +27,10 @@ const OAuth: React.FC = () => {
    getApps()
  }, [])

+  const handleShowModalCreate = () => setShowModalCreate(true)
+
+  const handleCloseModalCreate = () => setShowModalCreate(false)
+
  const handleAdd = async (name: string, redirect: string) => {
    const result = await fetch.post<App | Exception>('/oauth/clients', {
      name,
@ -106,11 +111,7 @@ const OAuth: React.FC = () => {

  return (
    <>
-      <button
-        className="btn btn-primary"
-        data-toggle="modal"
-        data-target="#modal-create"
-      >
+      <button className="btn btn-primary" onClick={handleShowModalCreate}>
        {trans('user.oauth.create')}
      </button>
      <div className="card mt-2">
@ -147,7 +148,11 @@ const OAuth: React.FC = () => {
          </table>
        </div>
      </div>
-      <ModalCreate onCreate={handleAdd} />
+      <ModalCreate
+        show={showModalCreate}
+        onCreate={handleAdd}
+        onClose={handleCloseModalCreate}
+      />
    </>
  )
 }
--- a/resources/assets/tests/components/Modal.test.tsx
+++ b/resources/assets/tests/components/Modal.test.tsx
@ -24,26 +24,15 @@ test('background color', () => {
  expect(container.querySelector('.modal-content')).toHaveClass('bg-primary')
 })

-test('forward ref', () => {
-  const ref = React.createRef<HTMLDivElement>()
-  render(<Modal ref={ref} show />)
-  expect(ref.current).not.toBeNull()
-})
-
 test('jQuery events', () => {
-  const ref = React.createRef<HTMLDivElement>()
-  const { getByText } = render(<Modal ref={ref} mode="confirm" show />)
+  const { getByText } = render(<Modal mode="confirm" show />)
  act(() => {
-    $(ref.current!)
-      .trigger('hide.bs.modal')
-      .trigger('hidden.bs.modal')
+    $('.modal').trigger('hidden.bs.modal')
  })

  fireEvent.click(getByText(trans('general.cancel')))
  act(() => {
-    $(ref.current!)
-      .trigger('hide.bs.modal')
-      .trigger('hidden.bs.modal')
+    $('.modal').trigger('hidden.bs.modal')
  })
 })

@ -280,3 +269,23 @@ describe('"prompt" mode', () => {
    expect(reject).not.toBeCalled()
  })
 })
+
+describe('"onClose" event', () => {
+  it('button confirm', () => {
+    const mock = jest.fn()
+    const { getByText } = render(<Modal show mode="confirm" onClose={mock} />)
+    fireEvent.click(getByText(trans('general.confirm')))
+    jest.runAllTimers()
+    $('.modal').trigger('hidden.bs.modal')
+    expect(mock).toBeCalled()
+  })
+
+  it('button cancel', () => {
+    const mock = jest.fn()
+    const { getByText } = render(<Modal show mode="confirm" onClose={mock} />)
+    fireEvent.click(getByText(trans('general.cancel')))
+    jest.runAllTimers()
+    $('.modal').trigger('hidden.bs.modal')
+    expect(mock).toBeCalled()
+  })
+})
--- a/resources/assets/tests/scripts/modal.test.ts
+++ b/resources/assets/tests/scripts/modal.test.ts
@ -1,21 +1,13 @@
-import $ from 'jquery'
-import { act } from 'react-dom/test-utils'
 import { trans } from '@/scripts/i18n'
 import { showModal } from '@/scripts/modal'

 test('show modal', async () => {
  process.nextTick(() => {
-    expect(
-      document.querySelector('.modal-title')!.textContent,
-    ).toBe(trans('general.tip'))
+    expect(document.querySelector('.modal-title')!.textContent).toBe(
+      trans('general.tip'),
+    )
    document.querySelector<HTMLButtonElement>('.btn-primary')!.click()
  })
  const { value } = await showModal()
  expect(value).toBe('')
-
-  act(() => {
-    $('.modal').trigger('hidden.bs.modal')
-    jest.runAllTimers()
-  })
-  expect(document.querySelector('.modal')).toBeNull()
 })