From daf69a1f277a6310a5da7b1c41dce4b2de1bba34 Mon Sep 17 00:00:00 2001 From: Dan Thompson Date: Thu, 9 May 2024 00:51:12 -0500 Subject: [PATCH 1/5] Avoid `scrollToIndex` when using native scrollbar in `ComboBox` --- .../src/components/combobox/combobox.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index bab5dd771..f83bea1b6 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -1688,6 +1688,14 @@ function OptionsFn( event.preventDefault() }) + // When the user scrolls **using the native scrollbar**, the only event fired is + // the onScroll event. We need to make sure to set the current activation + // trigger to pointer, in order to let them scroll through the list. + let handleScroll = useEvent(() => { + if (isMobile()) return + actions.setActivationTrigger(ActivationTrigger.Pointer) + }) + let ourProps = mergeProps(anchor ? getFloatingPanelProps() : {}, { 'aria-labelledby': labelledBy, role: 'listbox', @@ -1702,6 +1710,7 @@ function OptionsFn( } as CSSProperties, onWheel: handleWheel, onMouseDown: handleMouseDown, + onScroll: handleScroll, }) // Map the children in a scrollable container when virtualization is enabled From 779768d380df28ef9c195692e66923f945167467 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 28 May 2024 18:29:06 +0200 Subject: [PATCH 2/5] format comment --- .../@headlessui-react/src/components/combobox/combobox.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index f83bea1b6..083041e2f 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -1688,8 +1688,8 @@ function OptionsFn( event.preventDefault() }) - // When the user scrolls **using the native scrollbar**, the only event fired is - // the onScroll event. We need to make sure to set the current activation + // When the user scrolls **using the native scrollbar**, the only event fired + // is the onScroll event. We need to make sure to set the current activation // trigger to pointer, in order to let them scroll through the list. let handleScroll = useEvent(() => { if (isMobile()) return From 2f9aeb39ae8931db77c9f446b307716caf74f6e8 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 29 May 2024 14:01:29 +0200 Subject: [PATCH 3/5] set `ActivationTrigger` in mousedown If you use the scrollbar, a `mousedown` and `pointerdown` event is fired before you start interacting with the scrollbar. If you use the `scroll` event, then the callback is fired while scrolling. To improve performance a bit more, we will set the activation trigger in the `mousedown` event because we only need to set it once. If you are done scrolling, we don't reset it (we didn't do that before either), but instead we rely on other events to override the trigger (e.g.: you start using the keyboard). The big benefit is that this now only calls the `setActivationTrigger` once, instead of `n` times with the same value. --- .../src/components/combobox/combobox.tsx | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 083041e2f..09296ddcd 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -1676,23 +1676,20 @@ function OptionsFn( actions.setActivationTrigger(ActivationTrigger.Pointer) }) - // When clicking inside of the scrollbar, a `click` event will be triggered on - // the focusable element _below_ the scrollbar. If you use a `` - // inside of a ``, clicking the scrollbar of the `` - // will move focus to the `` which blurs the `` and - // closes the ``. - // - // Preventing the default behavior in the `mousedown` event (which happens - // before `click`) will prevent this issue because the `click` never fires. let handleMouseDown = useEvent((event: ReactMouseEvent) => { + // When clicking inside of the scrollbar, a `click` event will be triggered + // on the focusable element _below_ the scrollbar. If you use a `` + // inside of a ``, clicking the scrollbar of the `` + // will move focus to the `` which blurs the `` and + // closes the ``. + // + // Preventing the default behavior in the `mousedown` event (which happens + // before `click`) will prevent this issue because the `click` never fires. event.preventDefault() - }) - // When the user scrolls **using the native scrollbar**, the only event fired - // is the onScroll event. We need to make sure to set the current activation - // trigger to pointer, in order to let them scroll through the list. - let handleScroll = useEvent(() => { - if (isMobile()) return + // When the user clicks in the ``, we want to make sure that we + // set the activation trigger to `pointer` to prevent auto scrolling to the + // active option while the user is scrolling. actions.setActivationTrigger(ActivationTrigger.Pointer) }) @@ -1710,7 +1707,6 @@ function OptionsFn( } as CSSProperties, onWheel: handleWheel, onMouseDown: handleMouseDown, - onScroll: handleScroll, }) // Map the children in a scrollable container when virtualization is enabled From 4e34771f192d32ef6a843e70d7fcf6e3e8bee2e5 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 29 May 2024 14:03:59 +0200 Subject: [PATCH 4/5] optimize `onWheel` to only trigger once This is a similar performance trick as before. We only need to set the activationTrigger once, so there is no need to keep calling this function for no reason. --- .../@headlessui-react/src/components/combobox/combobox.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 09296ddcd..638f2e877 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -1671,7 +1671,7 @@ function OptionsFn( }, [data]) // When the user scrolls **using the mouse** (so scroll event isn't appropriate) - // we want to make sure that the current activation trigger is set to pointer + // we want to make sure that the current activation trigger is set to pointer. let handleWheel = useEvent(() => { actions.setActivationTrigger(ActivationTrigger.Pointer) }) @@ -1705,7 +1705,7 @@ function OptionsFn( '--input-width': useElementSize(data.inputRef, true).width, '--button-width': useElementSize(data.buttonRef, true).width, } as CSSProperties, - onWheel: handleWheel, + onWheel: data.activationTrigger === ActivationTrigger.Pointer ? undefined : handleWheel, onMouseDown: handleMouseDown, }) From 8c53bc1beea8e4a2b353089bbd5d2ea10661707a Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 29 May 2024 14:14:01 +0200 Subject: [PATCH 5/5] update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 76c0fc5f2..b8eab99a1 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Keep `` open when clicking scrollbar in `` ([#3249](https://github.com/tailwindlabs/headlessui/pull/3249)) - Merge incoming `style` prop on `ComboboxOptions`, `ListboxOptions`, `MenuItems`, and `PopoverPanel` components ([#3250](https://github.com/tailwindlabs/headlessui/pull/3250)) - Prevent focus on `` when it is `disabled` ([#3251](https://github.com/tailwindlabs/headlessui/pull/3251)) +- Fix visual jitter in `Combobox` component when using native scrollbar ([#3190](https://github.com/tailwindlabs/headlessui/pull/3190)) ## [2.0.4] - 2024-05-25