-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Freeze values as soon as possible #3802
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
Merged
Merged
+57
−25
Conversation
This file contains hidden or 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
We are only interested in CSS Transitions not Animations in this case. We are also interested in the ones that are not `finished` yet. We implemented this in Elements as well.
The moment we select an option in single value mode we will freeze the state immediately. This means that we don't have to rely on additional re-renders to freeze the value which could be too late. It's too late when between the `onChange` and `closeListbox` a re-render happens with the new value. Because then we would freeze the new value which would be wrong. This also slightly refactors the `selectActiveOption` and `selectOption` actions.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
RobinMalfait
commented
Sep 25, 2025
| done() { | ||
| if (cancelledRef.current) { | ||
| if (typeof element.getAnimations === 'function' && element.getAnimations().length > 0) { | ||
| if (hasPendingTransitions(element)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ported this from Elements because during my debugging I noticed that sometimes the transitions themselves were glitching when the component re-rendered with the new value while a transition was in progress.
packages/@headlessui-react/src/components/listbox/listbox-machine.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Jordan Pittman <[email protected]>
packages/@headlessui-react/src/components/listbox/listbox-machine.ts
Outdated
Show resolved
Hide resolved
We don't have to know whether we are transitioning or not. Because we will always freeze the value in single value mode.
Didn't do that initially because this exact block exists somewhere else, but the `else if` is a little bit cleaner syntax wise.
thecrypticace
approved these changes
Sep 25, 2025
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.
This PR fixes an issue with the
Listboxcomponent where we didn't freeze the value soon enough.This happens when state lives in the parent, and is updated via an
onChange.What is currently happening:
onChangewith the new valueThe problem is that calling the
onChangeupdates the value in the parent, and the component re-renders with the new value. At the time we freeze the value, we already received the new value so we are freezing the incorrect value. This causes a visual glitch. See reproduction: tailwindlabs/tailwind-plus-issues#1761This PR fixes that by changing the order a little bit so we freeze the value as early as possible.
So now, when the user clicks on an option, we trigger a
SelectOptionaction. This will track whether we should freeze the value or not in state immediately. After that, we call theonChange, and then close the listbox.Since we know we want to freeze the value before calling
onChange, we can be sure we are freezing the correct (old) value.Test plan
Made a little video but with a duration of 1000 instead of 100 so you can clearly see the old value and no visual jumps while the listbox is closing.
Screen.Recording.2025-09-25.at.14.33.49.mov
Fixes: tailwindlabs/tailwind-plus-issues#1761