Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix visual jitter in Combobox component when using native scrollbar #3190

Merged

Conversation

danthomps
Copy link
Contributor

@danthomps danthomps commented May 9, 2024

Fixes #3189

Bug

The dropdown will jitter when using the native scrollbar before entering the dropdown list with your cursor or using the mouse wheel to scroll.

This is because the ActivationTrigger has not been set to pointer. As a result, scrollToIndex is called while you are attempting to scroll, leading to the rubberband effect.

How to Reproduce

If you are on macOS, go to System Settings and set Show Scroll Bars to Always.

It can be difficult to reproduce without this, as you must hover over the native scroll bar first avoid triggering any React events.

tw native scroll (Optimized)

The bug on the Tailwind blog demo

tw-blog-scroll.mp4

Occasionally, React can render the scrolling quickly enough to make the rubberbanding non-visible, making it somewhat challenging to consistently reproduce on the blog post.

Reproducing on the Playground

Adding a 250ms artificial delay on the scrolling makes it much more obvious.

If you scroll with your mouse wheel or move your pointer inside the dropdown, the issue will resolve itself. The bug only occurs when you enter native scrollbar first and interact with it directly.

tw-combobox-scroll-bug (Optimized)

native-scroll-delay.mp4

A Fix

Adding a onScroll handler that ignores mobile and sets the ActivationTrigger to pointer.

native-scroll-fix.mp4

Feel free to discard any or all of this fix or improve it further. If nothing else, this demonstrates how to reproduce it.

Copy link

vercel bot commented May 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
headlessui-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 29, 2024 0:15am
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 29, 2024 0:15am

@@ -1675,6 +1683,7 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
'--button-width': useElementSize(data.buttonRef, true).width,
} as CSSProperties,
onWheel: handleWheel,
onScroll: handleScroll,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chose to use an onScroll event as none of the other events would fire if the first thing we move our mouse into is the native scrollbar.

@danthomps danthomps marked this pull request as ready for review May 9, 2024 05:57
@RobinMalfait RobinMalfait self-assigned this May 29, 2024
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.
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.
@RobinMalfait RobinMalfait force-pushed the fix/combobox-scrollbar-jitter branch from 86c8a82 to 4e34771 Compare May 29, 2024 12:12
@RobinMalfait RobinMalfait changed the title Fix jitter in Combobox when using native scrollbar Fix visual jitter in Combobox component when using native scrollbar May 29, 2024
@RobinMalfait
Copy link
Member

RobinMalfait commented May 29, 2024

Hey!

Thank you for this PR, I really appreciate it.

I made some changes, here is a quick breakdown:

  1. I rebased on top of the main branch to be in sync with the latest changes
  2. I moved the setActivationTrigger to the mousedown event. The reason is twofold:
    • The mousedown and pointerdown events will be triggered (once) when you click in the sidebar.
    • The scroll event triggered the callback n times, but the code doesn't rely on any information from that event (such as scroll position). The only thing we need to do right now is to set the activation trigger. Doing it in the mousedown event means that we only do it once instead of n times while still fixing the jittery behavior.

Thanks again for your initial work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Combobox scroll jitter when scrolling long list after an option is selected
2 participants