[internal] Move enabled parameter in hooks to first argument (#3245)

* move `enabled` parameter in hooks to front Whenever a hook requires an `enabled` state, the `enabled` parameter is moved to the front. Initially this was the last argument and enabled by default but everywhere that we use these hooks we have to pass a dedicated boolean anyway. This makes sure these hooks follow a similar pattern. Bonus points because Prettier can now improve formatting the usage of these hooks. The reason why is because there is no additional argument after the potential last callback. Before: ```ts let enabled = data.__demoMode ? false : modal && data.comboboxState === ComboboxState.Open useInertOthers( { allowed: useEvent(() => [ data.inputRef.current, data.buttonRef.current, data.optionsRef.current, ]), }, enabled ) ``` After: ```ts let enabled = data.__demoMode ? false : modal && data.comboboxState === ComboboxState.Open useInertOthers(enabled, { allowed: useEvent(() => [ data.inputRef.current, data.buttonRef.current, data.optionsRef.current, ]), }) ``` Much better! * inline variables
tailwindlabs · May 27, 2024 · 1ee4cfd · 1ee4cfd
1 parent 7be23e5
commit 1ee4cfd
Show file tree

Hide file tree

Showing 13 changed files with 120 additions and 135 deletions.
diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx
@@ -770,10 +770,9 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
   }, [data])
 
   // Handle outside click
-  useOutsideClick(
-    [data.buttonRef, data.inputRef, data.optionsRef],
-    () => actions.closeCombobox(),
-    data.comboboxState === ComboboxState.Open
+  let outsideClickEnabled = data.comboboxState === ComboboxState.Open
+  useOutsideClick(outsideClickEnabled, [data.buttonRef, data.inputRef, data.optionsRef], () =>
+    actions.closeCombobox()
   )
 
   let slot = useMemo(() => {
@@ -1623,25 +1622,25 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
   })()
 
   // Ensure we close the combobox as soon as the input becomes hidden
-  useOnDisappear(data.inputRef, actions.closeCombobox, visible)
+  useOnDisappear(visible, data.inputRef, actions.closeCombobox)
 
   // Enable scroll locking when the combobox is visible, and `modal` is enabled
-  useScrollLock(
-    ownerDocument,
-    data.__demoMode ? false : modal && data.comboboxState === ComboboxState.Open
-  )
+  let scrollLockEnabled = data.__demoMode
+    ? false
+    : modal && data.comboboxState === ComboboxState.Open
+  useScrollLock(scrollLockEnabled, ownerDocument)
 
   // Mark other elements as inert when the combobox is visible, and `modal` is enabled
-  useInertOthers(
-    {
-      allowed: useEvent(() => [
-        data.inputRef.current,
-        data.buttonRef.current,
-        data.optionsRef.current,
-      ]),
-    },
-    data.__demoMode ? false : modal && data.comboboxState === ComboboxState.Open
-  )
+  let inertOthersEnabled = data.__demoMode
+    ? false
+    : modal && data.comboboxState === ComboboxState.Open
+  useInertOthers(inertOthersEnabled, {
+    allowed: useEvent(() => [
+      data.inputRef.current,
+      data.buttonRef.current,
+      data.optionsRef.current,
+    ]),
+  })
 
   useIsoMorphicEffect(() => {
     data.optionsPropsRef.current.static = props.static ?? false
@@ -1650,9 +1649,8 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
     data.optionsPropsRef.current.hold = hold
   }, [data.optionsPropsRef, hold])
 
-  useTreeWalker({
+  useTreeWalker(data.comboboxState === ComboboxState.Open, {
     container: data.optionsRef.current,
-    enabled: data.comboboxState === ComboboxState.Open,
     accept(node) {
       if (node.getAttribute('role') === 'option') return NodeFilter.FILTER_REJECT
       if (node.hasAttribute('role')) return NodeFilter.FILTER_SKIP

diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx
@@ -260,43 +260,36 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
     usesOpenClosedState !== null ? (usesOpenClosedState & State.Closing) === State.Closing : false
 
   // Ensure other elements can't be interacted with
-  let inertEnabled = (() => {
+  let inertOthersEnabled = (() => {
+    if (__demoMode) return false
     // Only the top-most dialog should be allowed, all others should be inert
     if (hasNestedDialogs) return false
     if (isClosing) return false
     return enabled
   })()
-
-  useInertOthers(
-    {
-      allowed: useEvent(() => [
-        // Allow the headlessui-portal of the Dialog to be interactive. This
-        // contains the current dialog and the necessary focus guard elements.
-        internalDialogRef.current?.closest<HTMLElement>('[data-headlessui-portal]') ?? null,
-      ]),
-      disallowed: useEvent(() => [
-        // Disallow the "main" tree root node
-        mainTreeNodeRef.current?.closest<HTMLElement>('body > *:not(#headlessui-portal-root)') ??
-          null,
-      ]),
-    },
-    __demoMode ? false : inertEnabled
-  )
+  useInertOthers(inertOthersEnabled, {
+    allowed: useEvent(() => [
+      // Allow the headlessui-portal of the Dialog to be interactive. This
+      // contains the current dialog and the necessary focus guard elements.
+      internalDialogRef.current?.closest<HTMLElement>('[data-headlessui-portal]') ?? null,
+    ]),
+    disallowed: useEvent(() => [
+      // Disallow the "main" tree root node
+      mainTreeNodeRef.current?.closest<HTMLElement>('body > *:not(#headlessui-portal-root)') ??
+        null,
+    ]),
+  })
 
   // Close Dialog on outside click
   let outsideClickEnabled = (() => {
     if (!enabled) return false
     if (hasNestedDialogs) return false
     return true
   })()
-  useOutsideClick(
-    resolveRootContainers,
-    (event) => {
-      event.preventDefault()
-      close()
-    },
-    outsideClickEnabled
-  )
+  useOutsideClick(outsideClickEnabled, resolveRootContainers, (event) => {
+    event.preventDefault()
+    close()
+  })
 
   // Handle `Escape` to close
   let escapeToCloseEnabled = (() => {
@@ -335,10 +328,10 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
     if (hasParentDialog) return false
     return true
   })()
-  useScrollLock(ownerDocument, __demoMode ? false : scrollLockEnabled, resolveRootContainers)
+  useScrollLock(scrollLockEnabled, ownerDocument, resolveRootContainers)
 
   // Ensure we close the dialog as soon as the dialog itself becomes hidden
-  useOnDisappear(internalDialogRef, close, dialogState === DialogStates.Open)
+  useOnDisappear(enabled, internalDialogRef, close)
 
   let [describedby, DescriptionProvider] = useDescriptions()
 

diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx
@@ -560,18 +560,15 @@ function ListboxFn<
   }, [data])
 
   // Handle outside click
-  useOutsideClick(
-    [data.buttonRef, data.optionsRef],
-    (event, target) => {
-      dispatch({ type: ActionTypes.CloseListbox })
+  let outsideClickEnabled = data.listboxState === ListboxStates.Open
+  useOutsideClick(outsideClickEnabled, [data.buttonRef, data.optionsRef], (event, target) => {
+    dispatch({ type: ActionTypes.CloseListbox })
 
-      if (!isFocusableElement(target, FocusableMode.Loose)) {
-        event.preventDefault()
-        data.buttonRef.current?.focus()
-      }
-    },
-    data.listboxState === ListboxStates.Open
-  )
+    if (!isFocusableElement(target, FocusableMode.Loose)) {
+      event.preventDefault()
+      data.buttonRef.current?.focus()
+    }
+  })
 
   let slot = useMemo(() => {
     return {
@@ -927,19 +924,21 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
   })()
 
   // Ensure we close the listbox as soon as the button becomes hidden
-  useOnDisappear(data.buttonRef, actions.closeListbox, visible)
+  useOnDisappear(visible, data.buttonRef, actions.closeListbox)
 
   // Enable scroll locking when the listbox is visible, and `modal` is enabled
-  useScrollLock(
-    ownerDocument,
-    data.__demoMode ? false : modal && data.listboxState === ListboxStates.Open
-  )
+  let scrollLockEnabled = data.__demoMode
+    ? false
+    : modal && data.listboxState === ListboxStates.Open
+  useScrollLock(scrollLockEnabled, ownerDocument)
 
   // Mark other elements as inert when the listbox is visible, and `modal` is enabled
-  useInertOthers(
-    { allowed: useEvent(() => [data.buttonRef.current, data.optionsRef.current]) },
-    data.__demoMode ? false : modal && data.listboxState === ListboxStates.Open
-  )
+  let inertOthersEnabled = data.__demoMode
+    ? false
+    : modal && data.listboxState === ListboxStates.Open
+  useInertOthers(inertOthersEnabled, {
+    allowed: useEvent(() => [data.buttonRef.current, data.optionsRef.current]),
+  })
 
   let initialOption = useRef<number | null>(null)
 
@@ -970,7 +969,8 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
   //
   // This can be solved by only transitioning the `opacity` instead of everything, but if you _do_
   // want to transition the y-axis for example you will run into the same issue again.
-  let didButtonMove = useDidElementMove(data.buttonRef, data.listboxState !== ListboxStates.Open)
+  let didElementMoveEnabled = data.listboxState !== ListboxStates.Open
+  let didButtonMove = useDidElementMove(didElementMoveEnabled, data.buttonRef)
 
   // Now that we know that the button did move or not, we can either disable the panel and all of
   // its transitions, or rely on the `visible` state to hide the panel whenever necessary.

diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx
@@ -388,18 +388,15 @@ function MenuFn<TTag extends ElementType = typeof DEFAULT_MENU_TAG>(
   let menuRef = useSyncRefs(ref)
 
   // Handle outside click
-  useOutsideClick(
-    [buttonRef, itemsRef],
-    (event, target) => {
-      dispatch({ type: ActionTypes.CloseMenu })
+  let outsideClickEnabled = menuState === MenuStates.Open
+  useOutsideClick(outsideClickEnabled, [buttonRef, itemsRef], (event, target) => {
+    dispatch({ type: ActionTypes.CloseMenu })
 
-      if (!isFocusableElement(target, FocusableMode.Loose)) {
-        event.preventDefault()
-        buttonRef.current?.focus()
-      }
-    },
-    menuState === MenuStates.Open
-  )
+    if (!isFocusableElement(target, FocusableMode.Loose)) {
+      event.preventDefault()
+      buttonRef.current?.focus()
+    }
+  })
 
   let close = useEvent(() => {
     dispatch({ type: ActionTypes.CloseMenu })
@@ -624,19 +621,19 @@ function ItemsFn<TTag extends ElementType = typeof DEFAULT_ITEMS_TAG>(
   })()
 
   // Ensure we close the menu as soon as the button becomes hidden
-  useOnDisappear(state.buttonRef, () => dispatch({ type: ActionTypes.CloseMenu }), visible)
+  useOnDisappear(visible, state.buttonRef, () => {
+    dispatch({ type: ActionTypes.CloseMenu })
+  })
 
   // Enable scroll locking when the menu is visible, and `modal` is enabled
-  useScrollLock(
-    ownerDocument,
-    state.__demoMode ? false : modal && state.menuState === MenuStates.Open
-  )
+  let scrollLockEnabled = state.__demoMode ? false : modal && state.menuState === MenuStates.Open
+  useScrollLock(scrollLockEnabled, ownerDocument)
 
   // Mark other elements as inert when the menu is visible, and `modal` is enabled
-  useInertOthers(
-    { allowed: useEvent(() => [state.buttonRef.current, state.itemsRef.current]) },
-    state.__demoMode ? false : modal && state.menuState === MenuStates.Open
-  )
+  let inertOthersEnabled = state.__demoMode ? false : modal && state.menuState === MenuStates.Open
+  useInertOthers(inertOthersEnabled, {
+    allowed: useEvent(() => [state.buttonRef.current, state.itemsRef.current]),
+  })
 
   // We keep track whether the button moved or not, we only check this when the menu state becomes
   // closed. If the button moved, then we want to cancel pending transitions to prevent that the
@@ -647,7 +644,8 @@ function ItemsFn<TTag extends ElementType = typeof DEFAULT_ITEMS_TAG>(
   //
   // This can be solved by only transitioning the `opacity` instead of everything, but if you _do_
   // want to transition the y-axis for example you will run into the same issue again.
-  let didButtonMove = useDidElementMove(state.buttonRef, state.menuState !== MenuStates.Open)
+  let didButtonMoveEnabled = state.menuState !== MenuStates.Open
+  let didButtonMove = useDidElementMove(didButtonMoveEnabled, state.buttonRef)
 
   // Now that we know that the button did move or not, we can either disable the panel and all of
   // its transitions, or rely on the `visible` state to hide the panel whenever necessary.
@@ -662,9 +660,8 @@ function ItemsFn<TTag extends ElementType = typeof DEFAULT_ITEMS_TAG>(
     container.focus({ preventScroll: true })
   }, [state.menuState, state.itemsRef, ownerDocument, state.itemsRef.current])
 
-  useTreeWalker({
+  useTreeWalker(state.menuState === MenuStates.Open, {
     container: state.itemsRef.current,
-    enabled: state.menuState === MenuStates.Open,
     accept(node) {
       if (node.getAttribute('role') === 'menuitem') return NodeFilter.FILTER_REJECT
       if (node.hasAttribute('role')) return NodeFilter.FILTER_SKIP

diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx
@@ -365,18 +365,15 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(
   )
 
   // Handle outside click
-  useOutsideClick(
-    root.resolveContainers,
-    (event, target) => {
-      dispatch({ type: ActionTypes.ClosePopover })
+  let outsideClickEnabled = popoverState === PopoverStates.Open
+  useOutsideClick(outsideClickEnabled, root.resolveContainers, (event, target) => {
+    dispatch({ type: ActionTypes.ClosePopover })
 
-      if (!isFocusableElement(target, FocusableMode.Loose)) {
-        event.preventDefault()
-        button?.focus()
-      }
-    },
-    popoverState === PopoverStates.Open
-  )
+    if (!isFocusableElement(target, FocusableMode.Loose)) {
+      event.preventDefault()
+      button?.focus()
+    }
+  })
 
   let close = useEvent(
     (
@@ -868,10 +865,13 @@ function PanelFn<TTag extends ElementType = typeof DEFAULT_PANEL_TAG>(
   })()
 
   // Ensure we close the popover as soon as the button becomes hidden
-  useOnDisappear(state.button, () => dispatch({ type: ActionTypes.ClosePopover }), visible)
+  useOnDisappear(visible, state.button, () => {
+    dispatch({ type: ActionTypes.ClosePopover })
+  })
 
   // Enable scroll locking when the popover is visible, and `modal` is enabled
-  useScrollLock(ownerDocument, state.__demoMode ? false : modal && visible)
+  let scrollLockEnabled = state.__demoMode ? false : modal && visible
+  useScrollLock(scrollLockEnabled, ownerDocument)
 
   let handleKeyDown = useEvent((event: ReactKeyboardEvent<HTMLButtonElement>) => {
     switch (event.key) {

diff --git a/packages/@headlessui-react/src/components/transition/transition.tsx b/packages/@headlessui-react/src/components/transition/transition.tsx
@@ -586,7 +586,7 @@ function TransitionRootFn<TTag extends ElementType = typeof DEFAULT_TRANSITION_C
   )
 
   // Ensure we change the tree state to hidden once the transition becomes hidden
-  useOnDisappear(internalTransitionRef, () => setState(TreeStates.Hidden))
+  useOnDisappear(show, internalTransitionRef, () => setState(TreeStates.Hidden))
 
   useIsoMorphicEffect(() => {
     if (show) {

diff --git a/packages/@headlessui-react/src/hooks/use-did-element-move.ts b/packages/@headlessui-react/src/hooks/use-did-element-move.ts
@@ -1,10 +1,7 @@
 import { useRef, type MutableRefObject } from 'react'
 import { useIsoMorphicEffect } from './use-iso-morphic-effect'
 
-export function useDidElementMove(
-  element: MutableRefObject<HTMLElement | null>,
-  enabled: boolean = true
-) {
+export function useDidElementMove(enabled: boolean, element: MutableRefObject<HTMLElement | null>) {
   let elementPosition = useRef({ left: 0, top: 0 })
   useIsoMorphicEffect(() => {
     let el = element.current

diff --git a/packages/@headlessui-react/src/hooks/use-inert-others.test.tsx b/packages/@headlessui-react/src/hooks/use-inert-others.test.tsx
@@ -13,7 +13,7 @@ it('should be possible to inert an element', async () => {
   function Example() {
     let ref = useRef(null)
     let [enabled, setEnabled] = useState(true)
-    useInertOthers({ disallowed: () => [ref.current] }, enabled)
+    useInertOthers(enabled, { disallowed: () => [ref.current] })
 
     return (
       <div ref={ref} id="main">
@@ -59,7 +59,7 @@ it('should not mark an element as inert when the hook is disabled', async () =>
   function Example() {
     let ref = useRef(null)
     let [enabled, setEnabled] = useState(false)
-    useInertOthers({ disallowed: () => [ref.current] }, enabled)
+    useInertOthers(enabled, { disallowed: () => [ref.current] })
 
     return (
       <div ref={ref} id="main">
@@ -95,7 +95,7 @@ it('should mark the element as not inert anymore, once all references are gone',
     let ref = useRef<HTMLDivElement | null>(null)
 
     let [enabled, setEnabled] = useState(false)
-    useInertOthers({ disallowed: () => [ref.current?.parentElement ?? null] }, enabled)
+    useInertOthers(enabled, { disallowed: () => [ref.current?.parentElement ?? null] })
 
     return (
       <div ref={ref}>
@@ -143,10 +143,9 @@ it('should mark the element as not inert anymore, once all references are gone',
 it('should be possible to mark everything but allowed containers as inert', async () => {
   function Example({ children }: { children: ReactNode }) {
     let [enabled, setEnabled] = useState(false)
-    useInertOthers(
-      { allowed: () => [document.getElementById('a-a-b')!, document.getElementById('a-a-c')!] },
-      enabled
-    )
+    useInertOthers(enabled, {
+      allowed: () => [document.getElementById('a-a-b')!, document.getElementById('a-a-c')!],
+    })
 
     return (
       <div>