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

Change default tag from div to Fragment on Transition components #3110

Merged
merged 10 commits into from
Apr 19, 2024

Conversation

RobinMalfait
Copy link
Collaborator

@RobinMalfait RobinMalfait commented Apr 18, 2024

This PR changes the default tag for the Transition, TransitionRoot, and TransitionChild components from div to Fragment.

The rational behind this change is that the Transition component is a component that "configures" the transition of its children but it was rendering its own div instead. If you want to transition the component you are wrapping, you required as={Fragment}. In practice this tends to be what happens most often.

Especially, now in Headless UI v2, a lot of components like Dialog, PopoverPanel, ListboxOptions, ComboboxOptions and MenuItems are portalled to a different place in the DOM. This means that the Transition is happening on a div with no children.

Before:

<Transition
  show={isOpen}
  className="bg-white rounded-lg"
  enter="transition duration-200 ease-out"
  enterFrom="scale-95 opacity-0"
  enterTo="scale-100 opacity-100"
  leave="transition duration-300 ease-out"
  leaveFrom="scale-100 opacity-100"
  leaveTo="scale-95 opacity-0"
>
  {/* ... */}
</Transition>

With this change:

  <Transition
    show={isOpen}
-   className="bg-white rounded-lg"
    enter="transition duration-200 ease-out"
    enterFrom="scale-95 opacity-0"
    enterTo="scale-100 opacity-100"
    leave="transition duration-300 ease-out"
    leaveFrom="scale-100 opacity-100"
    leaveTo="scale-95 opacity-0"
  >
+   <div className="bg-white rounded-lg">
      {/* ... */}
+   </div>
  </Transition>

Copy link

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

If the `<Transition />` component "root" is used as a root placeholder
(for state management) and not making actual transitions itself, then we
don't require a `ref` element.
+ add `className="…"` to some places to indicate that we _do_ want to
  perform a transition and thus have to fail if the `ref` is missing.
Also ensure that a ref is required if the `as` prop is provided and
it's not a `Fragment`
These tests were rendering a `Debug` element that didn't render any DOM
nodes. Adding `as="div"` ensures that we are forwarding the ref
correctly.
@RobinMalfait RobinMalfait force-pushed the change/default-transition-tags branch from fa41da9 to 99698a4 Compare April 18, 2024 23:55
@reinink reinink changed the title Change default Transition tag from div to Fragment Change default tag from div to Fragment on Transition components Apr 19, 2024
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.

None yet

3 participants