Fix jitter in Combobox when using native scrollbar
#3190
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
danthomps
commented
May 9, 2024
| @@ -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, | |||
There was a problem hiding this comment.
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
commented
May 9, 2024
| // trigger to pointer, in order to let them scroll through the list. | ||
| let handleScroll = useEvent(() => { | ||
| if (isMobile()) return | ||
| actions.setActivationTrigger(ActivationTrigger.Pointer) |
There was a problem hiding this comment.
The bug appears to only occur on desktop, so we're ignoring scroll reactions on mobile. An alternative approach could be to maintain state around mousedown, mouseup, and onScroll events, but this method allows us to avoid adding any additional useState functions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ActivationTriggerhas not been set to pointer. As a result, scrollToIndexis 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.
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.
native-scroll-delay.mp4
A Fix
Adding a
onScrollhandler that ignores mobile and sets theActivationTriggerto 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.