Fix enter transitions for the Transition component
#3074
Merged
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 ↗︎
|
I spend a lot of time trying to debug this, and I'm not 100% sure that I'm correct yet. However, what we used to do is apply the "before" set of classes, wait a frame and then apply the "after" set of classes which should trigger a transition. However, for some reason, applying the "before" classes already start a transition. My current thinking is that our component is doing a lot of work and therfore prematurely applying the classes and actually triggering the transition. For example, if we want to go from `opacity-0` to `opacity-100`, it looks like setting `opacity-0` is already transitioning (from 100% because that's the default value). Then, we set `opacity-100`, but our component was just going from 100 -> 0 and we were only at let's say 98%. It looks like we cancelled the transition and only went from 98% -> 100%. I can't fully explain it purely because I don't 100% get what's going on. However, this commit fixes it in a way where we first prepare the transition by explicitly cancelling all in-flight transitions. Once that is done, we can apply the "before" set of classes, then we can apply the "after" set of classes. This seems to work consistently (even though we have failing tests, might be a JSDOM issue). This tells me that at least parts of my initial thoughts are correct where some transition is already happening (for some reason). I'm not sure what the reason is exactly. Are we doing too much work and blocking the main thread? That would be my guess...
RobinMalfait
force-pushed
the
fix/enter-transitions
branch
from
4c845fd to
5fcd736
Compare
Instead of always ensuring that there is an event, let's use the `?.` operator and conditionally call the callbacks instead.
RobinMalfait
force-pushed
the
fix/enter-transitions
branch
from
b5c5652 to
b905bf8
Compare
…nsition.ts Co-authored-by: Jonathan Reinink <[email protected]>
…nsition.ts Co-authored-by: Jonathan Reinink <[email protected]>
…nsition.ts Co-authored-by: Jonathan Reinink <[email protected]>
…nsition.ts Co-authored-by: Jonathan Reinink <[email protected]>
Co-authored-by: Jonathan Reinink <[email protected]>
reinink
reviewed
Apr 5, 2024
packages/@headlessui-react/src/components/transition/transition.tsx
Outdated
Show resolved
Hide resolved
…n.tsx Co-authored-by: Jonathan Reinink <[email protected]>
reinink
reviewed
Apr 5, 2024
packages/@headlessui-react/src/components/transition/transition.tsx
Outdated
Show resolved
Hide resolved
…n.tsx Co-authored-by: Jonathan Reinink <[email protected]>
This was referenced Apr 5, 2024
This was referenced May 28, 2024
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 the enter transitions when using the
Transitioncomponent. We also simplify some internals such that it is easier to reason about.A few fixes went into this PR:
enterToorleaveToclasses are still present once the transition is complete. Otherwise it could happen that you transition frombg-red-500tobg-blue-500and once it's done, you end up withbg-red-500again.enter enterFromorleave leaveFrom) and last but not least, trigger a reflow to flush all the changes to the DOM at once which starts the transition.<Menu />component is opening) theleave leaveToclasses will be applied (as-if you are closing the<Menu />).Fixes: #2914
Fixes: #2991