Refactor

yuanqing · Oct 2, 2023 · 30c2606 · 30c2606
1 parent 72554ac
commit 30c2606
Show file tree

Hide file tree

Showing 49 changed files with 386 additions and 377 deletions.
diff --git a/packages/ui/src/components/banner/banner.tsx b/packages/ui/src/components/banner/banner.tsx
@@ -1,4 +1,4 @@
-import { ComponentChildren, h, JSX } from 'preact'
+import { ComponentChildren, h } from 'preact'
 
 import { createClassName } from '../../utilities/create-class-name.js'
 import { createComponent } from '../../utilities/create-component.js'
@@ -14,7 +14,7 @@ export type BannerVariant = 'success' | 'warning'
 export const Banner = createComponent<HTMLDivElement, BannerProps>(function (
   { children, icon, variant, ...rest },
   ref
-): JSX.Element {
+) {
   return (
     <div
       {...rest}

diff --git a/packages/ui/src/components/button/button.tsx b/packages/ui/src/components/button/button.tsx
@@ -1,4 +1,4 @@
-import { ComponentChildren, h, JSX } from 'preact'
+import { ComponentChildren, h } from 'preact'
 import { useCallback } from 'preact/hooks'
 
 import { Event, EventHandler } from '../../types/event-handler.js'
@@ -35,18 +35,17 @@ export const Button = createComponent<HTMLButtonElement, ButtonProps>(function (
     ...rest
   },
   ref
-): JSX.Element {
+) {
   const handleKeyDown = useCallback(
-    function (event: Event.onKeyDown<HTMLButtonElement>): void {
+    function (event: Event.onKeyDown<HTMLButtonElement>) {
       onKeyDown(event)
-      if (event.key !== 'Escape') {
-        return
-      }
-      if (propagateEscapeKeyDown === false) {
-        event.stopPropagation()
-      }
-      if (blurOnEscapeKeyDown === true) {
-        event.currentTarget.blur()
+      if (event.key === 'Escape') {
+        if (propagateEscapeKeyDown === false) {
+          event.stopPropagation()
+        }
+        if (blurOnEscapeKeyDown === true) {
+          event.currentTarget.blur()
+        }
       }
     },
     [blurOnEscapeKeyDown, onKeyDown, propagateEscapeKeyDown]
@@ -72,10 +71,8 @@ export const Button = createComponent<HTMLButtonElement, ButtonProps>(function (
         {...rest}
         ref={ref}
         disabled={disabled === true}
-        onClick={disabled === true || loading === true ? undefined : onClick}
-        onKeyDown={
-          disabled === true || loading === true ? undefined : handleKeyDown
-        }
+        onClick={loading === true ? undefined : onClick}
+        onKeyDown={handleKeyDown}
         tabIndex={0}
       >
         <div class={styles.children}>{children}</div>

diff --git a/packages/ui/src/components/checkbox/checkbox.tsx b/packages/ui/src/components/checkbox/checkbox.tsx
@@ -1,4 +1,4 @@
-import { ComponentChildren, h, JSX } from 'preact'
+import { ComponentChildren, h } from 'preact'
 import { useCallback } from 'preact/hooks'
 
 import { IconControlCheckboxChecked12 } from '../../icons/icon-12/icon-control-checkbox-checked-12.js'
@@ -32,27 +32,26 @@ export const Checkbox = createComponent<HTMLInputElement, CheckboxProps>(
       ...rest
     },
     ref
-  ): JSX.Element {
+  ) {
     const handleChange = useCallback(
-      function (event: Event.onChange<HTMLInputElement>): void {
-        const newValue = event.currentTarget.checked
-        onValueChange(newValue)
+      function (event: Event.onChange<HTMLInputElement>) {
         onChange(event)
+        const newValue = event.currentTarget.checked === true
+        onValueChange(newValue)
       },
       [onChange, onValueChange]
     )
 
     const handleKeyDown = useCallback(
-      function (event: Event.onKeyDown<HTMLInputElement>): void {
+      function (event: Event.onKeyDown<HTMLInputElement>) {
         onKeyDown(event)
-        if (event.key !== 'Escape') {
-          return
-        }
-        if (propagateEscapeKeyDown === false) {
-          event.stopPropagation()
-        }
-        if (blurOnEscapeKeyDown === true) {
-          event.currentTarget.blur()
+        if (event.key === 'Escape') {
+          if (propagateEscapeKeyDown === false) {
+            event.stopPropagation()
+          }
+          if (blurOnEscapeKeyDown === true) {
+            event.currentTarget.blur()
+          }
         }
       },
       [blurOnEscapeKeyDown, onKeyDown, propagateEscapeKeyDown]

diff --git a/packages/ui/src/components/disclosure/disclosure.tsx b/packages/ui/src/components/disclosure/disclosure.tsx
@@ -1,4 +1,4 @@
-import { ComponentChildren, Fragment, h, JSX } from 'preact'
+import { ComponentChildren, Fragment, h } from 'preact'
 import { useCallback } from 'preact/hooks'
 
 import { IconCaretRight16 } from '../../icons/icon-16/icon-caret-right-16.js'
@@ -29,18 +29,17 @@ export const Disclosure = createComponent<HTMLInputElement, DisclosureProps>(
       ...rest
     },
     ref
-  ): JSX.Element {
+  ) {
     const handleKeyDown = useCallback(
-      function (event: Event.onKeyDown<HTMLInputElement>): void {
+      function (event: Event.onKeyDown<HTMLInputElement>) {
         onKeyDown(event)
-        if (event.key !== 'Escape') {
-          return
-        }
-        if (propagateEscapeKeyDown === false) {
-          event.stopPropagation()
-        }
-        if (blurOnEscapeKeyDown === true) {
-          event.currentTarget.blur()
+        if (event.key === 'Escape') {
+          if (propagateEscapeKeyDown === false) {
+            event.stopPropagation()
+          }
+          if (blurOnEscapeKeyDown === true) {
+            event.currentTarget.blur()
+          }
         }
       },
       [blurOnEscapeKeyDown, onKeyDown, propagateEscapeKeyDown]

diff --git a/packages/ui/src/components/divider/divider.tsx b/packages/ui/src/components/divider/divider.tsx
@@ -1,10 +1,10 @@
-import { h, JSX } from 'preact'
+import { h } from 'preact'
 
 import { createComponent } from '../../utilities/create-component.js'
 import styles from './divider.module.css'
 
 export const Divider = createComponent<HTMLHRElement, Record<string, never>>(
-  function (rest, ref): JSX.Element {
+  function (rest, ref) {
     return <hr {...rest} ref={ref} class={styles.divider} />
   }
 )
diff --git a/packages/ui/src/components/dropdown/dropdown.tsx b/packages/ui/src/components/dropdown/dropdown.tsx
@@ -1,4 +1,4 @@
-import { ComponentChildren, h, JSX, RefObject } from 'preact'
+import { ComponentChildren, h, RefObject } from 'preact'
 import { useCallback, useRef, useState } from 'preact/hooks'
 
 import menuStyles from '../../css/menu.module.css'
@@ -58,7 +58,7 @@ export const Dropdown = createComponent<HTMLDivElement, DropdownProps>(
       ...rest
     },
     ref
-  ): JSX.Element {
+  ) {
     if (typeof icon === 'string' && icon.length !== 1) {
       throw new Error(`String \`icon\` must be a single character: "${icon}"`)
     }
@@ -91,11 +91,11 @@ export const Dropdown = createComponent<HTMLDivElement, DropdownProps>(
         setSelectedId: setSelectedId
       })
 
-    const triggerBlur = useCallback(function (): void {
+    const triggerBlur = useCallback(function () {
       getCurrentFromRef(rootElementRef).blur()
     }, [])
 
-    const triggerHideMenu = useCallback(function (): void {
+    const triggerHideMenu = useCallback(function () {
       setIsMenuVisible(false)
       setSelectedId(INVALID_ID)
     }, [])
@@ -166,7 +166,7 @@ export const Dropdown = createComponent<HTMLDivElement, DropdownProps>(
     )
 
     const handleRootKeyDown = useCallback(
-      function (event: Event.onKeyDown<HTMLDivElement>): void {
+      function (event: Event.onKeyDown<HTMLDivElement>) {
         const key = event.key
         if (key === 'ArrowUp' || key === 'ArrowDown') {
           if (isMenuVisible === false) {
@@ -239,7 +239,7 @@ export const Dropdown = createComponent<HTMLDivElement, DropdownProps>(
     [])
 
     const handleOptionChange = useCallback(
-      function (event: Event.onChange<HTMLInputElement>): void {
+      function (event: Event.onChange<HTMLInputElement>) {
         const id = event.currentTarget.getAttribute(
           ITEM_ID_DATA_ATTRIBUTE_NAME
         ) as string
@@ -343,10 +343,7 @@ export const Dropdown = createComponent<HTMLDivElement, DropdownProps>(
           ])}
           onMouseDown={handleMenuMouseDown}
         >
-          {options.map(function (
-            option: DropdownOption,
-            index: number
-          ): JSX.Element {
+          {options.map(function (option: DropdownOption, index: number) {
             if (typeof option === 'string') {
               return <hr key={index} class={menuStyles.optionSeparator} />
             }

diff --git a/packages/ui/src/components/file-upload/file-upload-button/file-upload-button.tsx b/packages/ui/src/components/file-upload/file-upload-button/file-upload-button.tsx
@@ -1,4 +1,4 @@
-import { ComponentChildren, h, JSX } from 'preact'
+import { ComponentChildren, h } from 'preact'
 import { useCallback } from 'preact/hooks'
 
 import { Event, EventHandler } from '../../../types/event-handler.js'
@@ -21,6 +21,7 @@ export interface FileUploadButtonProps
   multiple?: boolean
   onChange?: EventHandler.onChange<HTMLInputElement>
   onClick?: EventHandler.onClick<HTMLInputElement>
+  onMouseDown?: EventHandler.onMouseDown<HTMLInputElement>
   onSelectedFiles?: EventHandler.onSelectedFiles
   secondary?: boolean
 }
@@ -39,50 +40,58 @@ export const FileUploadButton = createComponent<
     multiple = false,
     onChange = noop,
     onClick = noop,
-    onSelectedFiles = noop,
     onKeyDown = noop,
+    onMouseDown = noop,
+    onSelectedFiles = noop,
     propagateEscapeKeyDown = true,
     secondary = false,
     ...rest
   },
   ref
-): JSX.Element {
+) {
   const handleChange = useCallback(
-    function (event: Event.onChange<HTMLInputElement>): void {
+    function (event: Event.onChange<HTMLInputElement>) {
       onChange(event)
-      if (disabled === true || loading === true) {
-        return
+      const fileList = event.currentTarget.files
+      if (fileList === null) {
+        throw new Error('`event.currentTarget.files` is `null`')
+      }
+      const files = parseFileList(fileList)
+      if (files.length > 0) {
+        onSelectedFiles(files)
       }
-      const files = Array.prototype.slice
-        .call(event.currentTarget.files)
-        .sort(fileComparator)
-      onSelectedFiles(files)
     },
-    [disabled, loading, onChange, onSelectedFiles]
+    [onChange, onSelectedFiles]
   )
 
   const handleClick = useCallback(
-    function (event: Event.onClick<HTMLInputElement>): void {
+    function (event: Event.onClick<HTMLInputElement>) {
       onClick(event)
-      if (disabled === true || loading === true) {
-        return
+      if (loading === true) {
+        event.preventDefault()
       }
+    },
+    [onClick, loading]
+  )
+
+  const handleMouseDown = useCallback(
+    function (event: Event.onClick<HTMLInputElement>) {
+      onMouseDown(event)
       event.currentTarget.focus()
     },
-    [disabled, loading, onClick]
+    [onMouseDown]
   )
 
   const handleKeyDown = useCallback(
-    function (event: Event.onKeyDown<HTMLInputElement>): void {
+    function (event: Event.onKeyDown<HTMLInputElement>) {
       onKeyDown(event)
-      if (event.key !== 'Escape') {
-        return
-      }
-      if (propagateEscapeKeyDown === false) {
-        event.stopPropagation()
-      }
-      if (blurOnEscapeKeyDown === true) {
-        event.currentTarget.blur()
+      if (event.key === 'Escape') {
+        if (propagateEscapeKeyDown === false) {
+          event.stopPropagation()
+        }
+        if (blurOnEscapeKeyDown === true) {
+          event.currentTarget.blur()
+        }
       }
     },
     [blurOnEscapeKeyDown, onKeyDown, propagateEscapeKeyDown]
@@ -121,6 +130,7 @@ export const FileUploadButton = createComponent<
         onChange={handleChange}
         onClick={handleClick}
         onKeyDown={handleKeyDown}
+        onMouseDown={handleMouseDown}
         tabIndex={0}
         title=""
         type="file"
@@ -131,3 +141,7 @@ export const FileUploadButton = createComponent<
     </div>
   )
 })
+
+function parseFileList(fileList: FileList): Array<File> {
+  return Array.prototype.slice.call(fileList).sort(fileComparator)
+}
diff --git a/.../components/file-upload/file-upload-button/stories/file-upload-button-default.stories.tsx b/.../components/file-upload/file-upload-button/stories/file-upload-button-default.stories.tsx
@@ -59,8 +59,8 @@ export const Loading = function () {
 }
 
 export const LoadingFocused = function () {
-  function handleSelectedFiles(files: Array<File>) {
-    console.log(files)
+  function handleSelectedFiles() {
+    throw new Error('This function should not be called')
   }
   return (
     <FileUploadButton

diff --git a/...omponents/file-upload/file-upload-button/stories/file-upload-button-secondary.stories.tsx b/...omponents/file-upload/file-upload-button/stories/file-upload-button-secondary.stories.tsx
@@ -60,8 +60,8 @@ export const Loading = function () {
 }
 
 export const LoadingFocused = function () {
-  function handleSelectedFiles(files: Array<File>) {
-    console.log(files)
+  function handleSelectedFiles() {
+    throw new Error('This function should not be called')
   }
   return (
     <FileUploadButton