Remove focus() from Listbox Option (#1218)

* cleanup auto-scrolling

We keep the actual container focused, so we don't require the invidiual
option to be focused as well. We do want to scroll it into view but
that's part of another piece of code.

Also cleaned up some manual `document.querySelector` calls now that we
keep track of a `ref`.

* update changelog

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Adjust active {item,option} index ([#1184](https://github.com/tailwindlabs/headlessui/pull/1184))
 - Only activate the `Tab` on mouseup ([#1192](https://github.com/tailwindlabs/headlessui/pull/1192))
 - Ignore "outside click" on removed elements ([#1193](https://github.com/tailwindlabs/headlessui/pull/1193))
+- Remove `focus()` from Listbox Option ([#1218](https://github.com/tailwindlabs/headlessui/pull/1218))
 
 ### Added
 
@@ -44,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix `Dialog` cycling ([#553](https://github.com/tailwindlabs/headlessui/pull/553))
 - Only activate the `Tab` on mouseup ([#1192](https://github.com/tailwindlabs/headlessui/pull/1192))
 - Ignore "outside click" on removed elements ([#1193](https://github.com/tailwindlabs/headlessui/pull/1193))
+- Remove `focus()` from Listbox Option ([#1218](https://github.com/tailwindlabs/headlessui/pull/1218))
 
 ### Added
 

packages/@headlessui-react/src/components/listbox/listbox.tsx

@@ -15,6 +15,7 @@ import React, {
   MouseEvent as ReactMouseEvent,
   MutableRefObject,
   Ref,
+  useEffect,
 } from 'react'
 
 import { useDisposables } from '../../hooks/use-disposables'
@@ -554,7 +555,7 @@ let Options = forwardRefWithAs(function Options<
     return state.listboxState === ListboxStates.Open
   })()
 
-  useIsoMorphicEffect(() => {
+  useEffect(() => {
     let container = state.optionsRef.current
     if (!container) return
     if (state.listboxState !== ListboxStates.Open) return
@@ -706,6 +707,17 @@ let Option = forwardRefWithAs(function Option<
   let internalOptionRef = useRef<HTMLElement | null>(null)
   let optionRef = useSyncRefs(ref, internalOptionRef)
 
+  useIsoMorphicEffect(() => {
+    if (state.listboxState !== ListboxStates.Open) return
+    if (!active) return
+    if (state.activationTrigger === ActivationTrigger.Pointer) return
+    let d = disposables()
+    d.requestAnimationFrame(() => {
+      internalOptionRef.current?.scrollIntoView?.({ block: 'nearest' })
+    })
+    return d.dispose
+  }, [internalOptionRef, active, state.listboxState, state.activationTrigger, /* We also want to trigger this when the position of the active item changes so that we can re-trigger the scrollIntoView */ state.activeOptionIndex])
+
   let bag = useRef<ListboxOptionDataRef['current']>({ disabled, value, domRef: internalOptionRef })
 
   useIsoMorphicEffect(() => {
@@ -715,8 +727,8 @@ let Option = forwardRefWithAs(function Option<
     bag.current.value = value
   }, [bag, value])
   useIsoMorphicEffect(() => {
-    bag.current.textValue = document.getElementById(id)?.textContent?.toLowerCase()
-  }, [bag, id])
+    bag.current.textValue = internalOptionRef.current?.textContent?.toLowerCase()
+  }, [bag, internalOptionRef])
 
   let select = useCallback(() => state.propsRef.current.onChange(value), [state.propsRef, value])
 
@@ -729,20 +741,8 @@ let Option = forwardRefWithAs(function Option<
     if (state.listboxState !== ListboxStates.Open) return
     if (!selected) return
     dispatch({ type: ActionTypes.GoToOption, focus: Focus.Specific, id })
-    document.getElementById(id)?.focus?.()
   }, [state.listboxState])
 
-  useIsoMorphicEffect(() => {
-    if (state.listboxState !== ListboxStates.Open) return
-    if (!active) return
-    if (state.activationTrigger === ActivationTrigger.Pointer) return
-    let d = disposables()
-    d.requestAnimationFrame(() => {
-      document.getElementById(id)?.scrollIntoView?.({ block: 'nearest' })
-    })
-    return d.dispose
-  }, [id, active, state.listboxState, state.activationTrigger, /* We also want to trigger this when the position of the active item changes so that we can re-trigger the scrollIntoView */ state.activeOptionIndex])
-
   let handleClick = useCallback(
     (event: { preventDefault: Function }) => {
       if (disabled) return event.preventDefault()

packages/@headlessui-vue/src/components/listbox/listbox.ts

@@ -473,8 +473,8 @@ export let ListboxOptions = defineComponent({
           event.preventDefault()
           event.stopPropagation()
           if (api.activeOptionIndex.value !== null) {
-            let { dataRef } = api.options.value[api.activeOptionIndex.value]
-            api.select(dataRef.value)
+            let activeOption = api.options.value[api.activeOptionIndex.value]
+            api.select(activeOption.dataRef.value)
           }
           api.closeListbox()
           nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true }))
@@ -592,7 +592,7 @@ export let ListboxOption = defineComponent({
       domRef: internalOptionRef,
     }))
     onMounted(() => {
-      let textValue = document.getElementById(id)?.textContent?.toLowerCase().trim()
+      let textValue = dom(internalOptionRef)?.textContent?.toLowerCase().trim()
       if (textValue !== undefined) dataRef.value.textValue = textValue
     })
 
@@ -606,7 +606,6 @@ export let ListboxOption = defineComponent({
           if (api.listboxState.value !== ListboxStates.Open) return
           if (!selected.value) return
           api.goToOption(Focus.Specific, id)
-          document.getElementById(id)?.focus?.()
         },
         { immediate: true }
       )
@@ -616,7 +615,7 @@ export let ListboxOption = defineComponent({
       if (api.listboxState.value !== ListboxStates.Open) return
       if (!active.value) return
       if (api.activationTrigger.value === ActivationTrigger.Pointer) return
-      nextTick(() => document.getElementById(id)?.scrollIntoView?.({ block: 'nearest' }))
+      nextTick(() => dom(internalOptionRef)?.scrollIntoView?.({ block: 'nearest' }))
     })
 
     function handleClick(event: MouseEvent) {