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
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Use native `useId` and `useSyncExternalStore` hooks ([#3092](https://github.com/tailwindlabs/headlessui/pull/3092))
- Use `absolute` as the default Floating UI strategy ([#3097](https://github.com/tailwindlabs/headlessui/pull/3097))
- Change default tags for `ListboxOptions`, `ListboxOption`, `ComboboxOptions`, `ComboboxOption` and `TabGroup` components ([#3109](https://github.com/tailwindlabs/headlessui/pull/3109))
- Change default `Transition` tag from `div` to `Fragment` ([#3110](https://github.com/tailwindlabs/headlessui/pull/3110))
reinink marked this conversation as resolved.
Show resolved Hide resolved

### Added

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ describe('Composition', () => {
<Transition>
<Debug name="Transition" fn={orderFn} />
<Disclosure.Panel>
<Transition.Child>
<Transition.Child as="div">
<Debug name="Transition.Child" fn={orderFn} />
</Transition.Child>
</Disclosure.Panel>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ describe('Composition', () => {
<Transition>
<Debug name="Transition" fn={orderFn} />
<Popover.Panel>
<Transition.Child>
<Transition.Child as="div">
<Debug name="Transition.Child" fn={orderFn} />
</Transition.Child>
</Popover.Panel>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,6 @@ exports[`Setup API shallow should passthrough all the props (that we do not use
</a>
`;

exports[`Setup API shallow should render a div and its children by default 1`] = `
<div>
Children
</div>
`;

exports[`Setup API shallow should render another component if the \`as\` prop is used and its children by default 1`] = `
<a>
Children
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { act as _act, fireEvent, render } from '@testing-library/react'
import React, { Fragment, useEffect, useLayoutEffect, useRef, useState } from 'react'
import React, { useEffect, useLayoutEffect, useRef, useState } from 'react'
import { getByText } from '../../test-utils/accessibility-assertions'
import { executeTimeline } from '../../test-utils/execute-timeline'
import { click } from '../../test-utils/interactions'
Expand All @@ -22,7 +22,7 @@ function nextFrame() {
it('should not steal the ref from the child', async () => {
let fn = jest.fn()
render(
<Transition show={true} as={Fragment}>
<Transition show={true}>
<div ref={fn}>...</div>
</Transition>
)
Expand All @@ -40,11 +40,6 @@ it('should render without crashing', () => {
)
})

it('should be possible to render a Transition without children', () => {
render(<Transition show={true} className="transition" />)
expect(document.getElementsByClassName('transition')).not.toBeNull()
})

it(
'should yell at us when we forget the required show prop',
suppressConsoleLogs(() => {
Expand All @@ -62,16 +57,15 @@ it(

describe('Setup API', () => {
describe('shallow', () => {
it('should render a div and its children by default', () => {
let { container } = render(<Transition show={true}>Children</Transition>)

expect(container.firstChild).toMatchSnapshot()
})

it('should passthrough all the props (that we do not use internally)', () => {
let { container } = render(
/**
* Renders a Fragment by default and forwards props. But not possible to
* type in TypeScript land. This is also discouraged, but it works.
*/
// @ts-expect-error
<Transition show={true} id="root" className="text-blue-400">
Children
<div>Children</div>
</Transition>
)

Expand Down Expand Up @@ -99,7 +93,11 @@ describe('Setup API', () => {
})

it('should render nothing when the show prop is false', () => {
let { container } = render(<Transition show={false}>Children</Transition>)
let { container } = render(
<Transition show={false}>
<div>Children</div>
</Transition>
)

expect(container.firstChild).toMatchSnapshot()
})
Expand All @@ -115,11 +113,7 @@ describe('Setup API', () => {
})

it('should be possible to use a render prop', () => {
let { container } = render(
<Transition show={true} as={Fragment}>
{() => <span>Children</span>}
</Transition>
)
let { container } = render(<Transition show={true}>{() => <span>Children</span>}</Transition>)

expect(container.firstChild).toMatchSnapshot()
})
Expand All @@ -135,7 +129,7 @@ describe('Setup API', () => {

expect(() => {
render(
<Transition show={true} as={Fragment}>
<Transition show={true} enter="duration-200">
{() => <Dummy />}
</Transition>
)
Expand All @@ -153,7 +147,9 @@ describe('Setup API', () => {
expect(() => {
render(
<div className="My Page">
<Transition.Child>Oops</Transition.Child>
<Transition.Child>
<div>Oops</div>
</Transition.Child>
</div>
)
}).toThrowErrorMatchingSnapshot()
Expand All @@ -163,7 +159,9 @@ describe('Setup API', () => {
it('should be possible to render a Transition.Child without children', () => {
render(
<Transition show={true}>
<Transition.Child className="transition" />
<Transition.Child>
<div className="transition" />
</Transition.Child>
</Transition>
)
expect(document.getElementsByClassName('transition')).not.toBeNull()
Expand All @@ -172,7 +170,9 @@ describe('Setup API', () => {
it('should be possible to use a Transition.Root and a Transition.Child', () => {
render(
<Transition.Root show={true}>
<Transition.Child className="transition" />
<Transition.Child>
<div className="transition" />
</Transition.Child>
</Transition.Root>
)
expect(document.getElementsByClassName('transition')).not.toBeNull()
Expand All @@ -182,8 +182,14 @@ describe('Setup API', () => {
let { container } = render(
<div className="My Page">
<Transition show={true}>
<Transition.Child>Sidebar</Transition.Child>
<Transition.Child>Content</Transition.Child>
<div>
<Transition.Child>
<div>Sidebar</div>
</Transition.Child>
<Transition.Child>
<div>Content</div>
</Transition.Child>
</div>
</Transition>
</div>
)
Expand All @@ -195,8 +201,10 @@ describe('Setup API', () => {
let { container } = render(
<div className="My Page">
<Transition show={true}>
<Transition.Child as="aside">Sidebar</Transition.Child>
<Transition.Child as="section">Content</Transition.Child>
<div>
<Transition.Child as="aside">Sidebar</Transition.Child>
<Transition.Child as="section">Content</Transition.Child>
</div>
</Transition>
</div>
)
Expand All @@ -221,8 +229,10 @@ describe('Setup API', () => {
let { container } = render(
<div className="My Page">
<Transition show={true}>
<Transition.Child as={Fragment}>{() => <aside>Sidebar</aside>}</Transition.Child>
<Transition.Child as={Fragment}>{() => <section>Content</section>}</Transition.Child>
<div>
<Transition.Child>{() => <aside>Sidebar</aside>}</Transition.Child>
<Transition.Child>{() => <section>Content</section>}</Transition.Child>
</div>
</Transition>
</div>
)
Expand All @@ -233,13 +243,11 @@ describe('Setup API', () => {
it('should be possible to use render props on the Transition and Transition.Child components', () => {
let { container } = render(
<div className="My Page">
<Transition show={true} as={Fragment}>
<Transition show={true}>
{() => (
<article>
<Transition.Child as={Fragment}>{() => <aside>Sidebar</aside>}</Transition.Child>
<Transition.Child as={Fragment}>
{() => <section>Content</section>}
</Transition.Child>
<Transition.Child>{() => <aside>Sidebar</aside>}</Transition.Child>
<Transition.Child>{() => <section>Content</section>}</Transition.Child>
</article>
)}
</Transition>
Expand All @@ -262,15 +270,43 @@ describe('Setup API', () => {
render(
<div className="My Page">
<Transition show={true}>
<Transition.Child as={Fragment}>{() => <Dummy>Sidebar</Dummy>}</Transition.Child>
<Transition.Child as={Fragment}>{() => <Dummy>Content</Dummy>}</Transition.Child>
<div>
<Transition.Child enter="duration-200">
{() => <Dummy>Sidebar</Dummy>}
</Transition.Child>
<Transition.Child enter="duration-200">
{() => <Dummy>Content</Dummy>}
</Transition.Child>
</div>
</Transition>
</div>
)
}).toThrowErrorMatchingSnapshot()
})
)

it(
'should not error when the `Transition` component does not contain any props that need to be forwarded',
suppressConsoleLogs(() => {
expect.assertions(1)

expect(() => {
render(
<div className="My Page">
<Transition show={true}>
<Transition.Child>
<div>Sidebar</div>
</Transition.Child>
<Transition.Child>
<div>Content</div>
</Transition.Child>
</Transition>
</div>
)
}).not.toThrow()
})
)

it(
'should yell at us when we forgot to forward a ref on the Transition component',
suppressConsoleLogs(() => {
Expand All @@ -283,7 +319,7 @@ describe('Setup API', () => {
expect(() => {
render(
<div className="My Page">
<Transition show={true} as={Fragment}>
<Transition show={true} enter="duration-200">
{() => (
<Dummy>
<Transition.Child>{() => <aside>Sidebar</aside>}</Transition.Child>
Expand Down Expand Up @@ -362,7 +398,7 @@ describe('Setup API', () => {
leaveFrom="leave-from"
leaveTo="leave-to"
>
Children
<div>Children</div>
</Transition>
)

Expand All @@ -387,7 +423,7 @@ describe('Setup API', () => {
leaveFrom="leave-from"
leaveTo="leave-to"
>
Children
<div>Children</div>
</Transition>
)
}
Expand Down Expand Up @@ -732,10 +768,10 @@ describe('Transitions', () => {

<Transition show={show}>
<Transition.Child leave="leave-fast" leaveFrom="leave-from" leaveTo="leave-to">
I am fast
<div>I am fast</div>
</Transition.Child>
<Transition.Child leave="leave-slow" leaveFrom="leave-from" leaveTo="leave-to">
I am slow
<div>I am slow</div>
</Transition.Child>
</Transition>

Expand Down Expand Up @@ -781,11 +817,11 @@ describe('Transitions', () => {
<Transition.Child leave="leave-fast" leaveFrom="leave-from" leaveTo="leave-to">
<span>I am fast</span>
<Transition show={show} leave="leave-slow">
I am my own root component and I don't talk to the parent
<div>I am my own root component and I don't talk to the parent</div>
</Transition>
</Transition.Child>
<Transition.Child leave="leave-slow" leaveFrom="leave-from" leaveTo="leave-to">
I am slow
<div>I am slow</div>
</Transition.Child>
</Transition>

Expand Down Expand Up @@ -953,12 +989,22 @@ describe('Events', () => {
<style>{`.child-2-2.leave { transition-duration: ${leaveDuration * 2.5}ms; }`}</style>

<Transition show={show} {...getProps('root')}>
<Transition.Child {...getProps('child-1')}>Child 1.</Transition.Child>
<Transition.Child {...getProps('child-2')}>
Child 2.
<Transition.Child {...getProps('child-2-1')}>Child 2.1.</Transition.Child>
<Transition.Child {...getProps('child-2-2')}>Child 2.2.</Transition.Child>
</Transition.Child>
<div>
<Transition.Child {...getProps('child-1')}>
<div>Child 1.</div>
</Transition.Child>
<Transition.Child {...getProps('child-2')}>
<div>
<div>Child 2.</div>
<Transition.Child {...getProps('child-2-1')}>
<div>Child 2.1.</div>
</Transition.Child>
<Transition.Child {...getProps('child-2-2')}>
<div>Child 2.2.</div>
</Transition.Child>
</div>
</Transition.Child>
</div>
</Transition>

<button
Expand Down Expand Up @@ -1104,7 +1150,9 @@ describe('Events', () => {
leave="leave-1"
leaveFrom="leave-from"
leaveTo="leave-to"
/>
>
<div />
</Transition.Child>
<Transition.Child
enter="enter-1"
enterFrom="enter-from"
Expand Down
Loading
Loading