Implement <Transition /> and <TransitionChild /> on top of data attributes
#3303
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
This will be used when we implement the `<Transition />` component purely using the `useTransitionData` information. But because there is a hierarchy between `<Transition />` and `<TransitionChild />` we need to know when transitions start and end.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
RobinMalfait
force-pushed
the
refactor/simplify-transition
branch
from
b6e207e
to
401d4fc
Compare
Due to a timing issue bug, we updated the snapshot tests in #3273 incorrectly so this commit fixes that which is why there are a lot of changes. Most tests have `show={true}` but not `appear` which means that they should _not_ transition which means that no data attributes should be present.
Now that we set state instead of mutating the DOM directly we need to wait a tiny bit and then we can trigger the transition to ensure a smooth transition.
RobinMalfait
force-pushed
the
refactor/simplify-transition
branch
from
401d4fc
to
1c7acd1
Compare
packages/@headlessui-react/src/components/transition/transition.tsx
Outdated
Show resolved
Hide resolved
thecrypticace
requested changes
Jun 19, 2024
There was a problem hiding this comment.
Lot of things work — including cancelling / uncancelling / etc… I've got a small change and a question but also noticed a few issues with the transition examples:
- Both Transition -> Nested examples are broken on the first transition
- Ditto about the color transition / Peek A Boo one
- Ditto for some of the Appear ones
- The Full Page transition initial element disappears on the first transition but then works fine for all subsequent transitions
These all seem to be related to behavior on the very first transition on an element but everything else seems fine.
…n.tsx Co-authored-by: Jordan Pittman <[email protected]>
This makes sure that the `Enter` state is applied initially when it has to. This also means that we can simplify the `prepareTransition` code again because we don't need to wait for the next microTask which made sure `TransitionState.Enter` was available.
thecrypticace
approved these changes
Jun 19, 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 is mainly a refactor to implement the
<Transition />and<TransitionChild />components on top of the data attribute implementation. The big difference is that there is no direct DOM mutation anymore, but just applying data attributes to the underlying element.This also means that the
<Transition />anduseTransitionData()don't have 2 separate paths for controlling the transition, but there is only a single source of truth.This PR also renames the
useTransitionData()touseTransition()and removed the originaluseTransition().Because of the refactor and not relying on direct DOM mutations we can use React to declaratively provide the correct state instead of doing it imperatively by mutating directly. Which in turn fixes an issue: #3277
From an API perspective nothing changes, it's purely an internal refactor.
Fixes: #3277