From 29a57e542a5628973d4f9a29d1846582184dfb85 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 9 Mar 2022 16:07:51 +0100 Subject: [PATCH 1/2] 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`. --- .../src/components/listbox/listbox.tsx | 30 +++++++++---------- .../src/components/listbox/listbox.ts | 9 +++--- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 58b239b71b..f4540e3503 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/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(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({ 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() diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.ts b/packages/@headlessui-vue/src/components/listbox/listbox.ts index 78f5233266..bf4e8321d8 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.ts +++ b/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) { From 5b5fbb6d0a22b25e07913a481f6de98028832f16 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 9 Mar 2022 16:18:33 +0100 Subject: [PATCH 2/2] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c85d81640..3cc279e8ea 100644 --- a/CHANGELOG.md +++ b/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