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

Implement <Transition /> and <TransitionChild /> on top of data attributes #3303

Merged
merged 13 commits into from
Jun 19, 2024

Conversation

RobinMalfait
Copy link
Member

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 /> and useTransitionData() 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() to useTransition() and removed the original useTransition().

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

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.
Copy link

vercel bot commented Jun 19, 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 Jun 19, 2024 9:41pm
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2024 9:41pm

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.
Copy link
Contributor

@thecrypticace thecrypticace left a comment

Choose a reason for hiding this comment

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

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:

  1. Both Transition -> Nested examples are broken on the first transition
  2. Ditto about the color transition / Peek A Boo one
  3. Ditto for some of the Appear ones
  4. 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.

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.
@RobinMalfait RobinMalfait merged commit 29e7d94 into main Jun 19, 2024
8 checks passed
@RobinMalfait RobinMalfait deleted the refactor/simplify-transition branch June 19, 2024 21:42
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.

Bug Conditional className when using DisclosurePanel inside Transition
2 participants