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

Fix components not properly closing when using the transition prop #3448

Merged
merged 5 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Nothing yet!
### Fixed

- Fix components not properly closing when using the `transition` prop ([#3448](https://github.com/tailwindlabs/headlessui/pull/3448))
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved

## [2.1.3] - 2024-08-23

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render } from '@testing-library/react'
import { render, waitFor } from '@testing-library/react'
import React, { Fragment, createElement, useEffect, useState } from 'react'
import {
ComboboxMode,
Expand Down Expand Up @@ -42,7 +42,13 @@ import {
} from '../../test-utils/interactions'
import { mockingConsoleLogs, suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
import { Transition } from '../transition/transition'
import { Combobox } from './combobox'
import {
Combobox,
ComboboxButton,
ComboboxInput,
ComboboxOption,
ComboboxOptions,
} from './combobox'

let NOOP = () => {}

Expand Down Expand Up @@ -6060,3 +6066,39 @@ describe('Form compatibility', () => {
])
})
})

describe('transitions', () => {
it(
'should be possible to close the Combobox when using the `transition` prop',
suppressConsoleLogs(async () => {
render(
<Combobox>
<ComboboxButton>Toggle</ComboboxButton>
<ComboboxInput />
<ComboboxOptions transition>
<ComboboxOption value="alice">Alice</ComboboxOption>
<ComboboxOption value="bob">Bob</ComboboxOption>
<ComboboxOption value="charlie">Charlie</ComboboxOption>
</ComboboxOptions>
</Combobox>
)

// Open the combobox
await click(getComboboxButton())

// Ensure the combobox is visible
assertCombobox({ state: ComboboxState.Visible })

// Close the combobox
await click(getComboboxButton())

// Wait for the transition to finish, and the combobox to close
await waitFor(() => {
assertComboboxList({ state: ComboboxState.InvisibleUnmounted })
})

// Ensure the input got the restored focus
assertActiveElement(getComboboxInput())
})
)
})
10 changes: 8 additions & 2 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1672,14 +1672,20 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
}

let [floatingRef, style] = useFloatingPanel(anchor)
let [localOptionsElement, setLocalOptionsElement] = useState<HTMLElement | null>(null)
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
let getFloatingPanelProps = useFloatingPanelProps()
let optionsRef = useSyncRefs(ref, anchor ? floatingRef : null, actions.setOptionsElement)
let optionsRef = useSyncRefs(
ref,
anchor ? floatingRef : null,
actions.setOptionsElement,
setLocalOptionsElement
)
let ownerDocument = useOwnerDocument(data.optionsElement)

let usesOpenClosedState = useOpenClosed()
let [visible, transitionData] = useTransition(
transition,
data.optionsElement,
localOptionsElement,
usesOpenClosedState !== null
? (usesOpenClosedState & State.Open) === State.Open
: data.comboboxState === ComboboxState.Open
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
import { render } from '@testing-library/react'
import React, { createElement, Suspense, useEffect, useRef } from 'react'
import { render, waitFor } from '@testing-library/react'
import React, { Suspense, createElement, useEffect, useRef } from 'react'
import {
DisclosureState,
assertActiveElement,
assertDisclosureButton,
assertDisclosurePanel,
DisclosureState,
getByText,
getDisclosureButton,
getDisclosurePanel,
} from '../../test-utils/accessibility-assertions'
import { click, focus, Keys, MouseButton, press } from '../../test-utils/interactions'
import { Keys, MouseButton, click, focus, press } from '../../test-utils/interactions'
import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
import { Transition } from '../transition/transition'
import { Disclosure } from './disclosure'
import { Disclosure, DisclosureButton, DisclosurePanel } from './disclosure'

jest.mock('../../hooks/use-id')

Expand Down Expand Up @@ -985,3 +985,40 @@ describe('Mouse interactions', () => {
})
)
})

describe('transitions', () => {
it(
'should be possible to close the Disclosure when using the `transition` prop',
suppressConsoleLogs(async () => {
render(
<Disclosure>
<DisclosureButton>Toggle</DisclosureButton>
<DisclosurePanel transition>Contents</DisclosurePanel>
</Disclosure>
)

// Focus the button
await focus(getDisclosureButton())

// Ensure the button is focused
assertActiveElement(getDisclosureButton())

// Open the disclosure
await click(getDisclosureButton())

// Ensure the disclosure is visible
assertDisclosurePanel({ state: DisclosureState.Visible })

// Close the disclosure
await click(getDisclosureButton())

// Wait for the transition to finish, and the disclosure to close
await waitFor(() => {
assertDisclosurePanel({ state: DisclosureState.InvisibleUnmounted })
})

// Ensure the button got the restored focus
assertActiveElement(getDisclosureButton())
})
)
})
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import React, {
useMemo,
useReducer,
useRef,
useState,
type ContextType,
type Dispatch,
type ElementType,
Expand Down Expand Up @@ -450,12 +451,14 @@ function PanelFn<TTag extends ElementType = typeof DEFAULT_PANEL_TAG>(
let [state, dispatch] = useDisclosureContext('Disclosure.Panel')
let { close } = useDisclosureAPIContext('Disclosure.Panel')
let mergeRefs = useMergeRefsFn()
let [localPanelElement, setLocalPanelElement] = useState<HTMLElement | null>(null)

let panelRef = useSyncRefs(
ref,
useEvent((element) => {
startTransition(() => dispatch({ type: ActionTypes.SetPanelElement, element }))
})
}),
setLocalPanelElement
)

useEffect(() => {
Expand All @@ -468,7 +471,7 @@ function PanelFn<TTag extends ElementType = typeof DEFAULT_PANEL_TAG>(
let usesOpenClosedState = useOpenClosed()
let [visible, transitionData] = useTransition(
transition,
state.panelElement,
localPanelElement,
usesOpenClosedState !== null
? (usesOpenClosedState & State.Open) === State.Open
: state.disclosureState === DisclosureStates.Open
Expand Down
45 changes: 43 additions & 2 deletions packages/@headlessui-react/src/components/listbox/listbox.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render } from '@testing-library/react'
import { render, waitFor } from '@testing-library/react'
import React, { createElement, useEffect, useState } from 'react'
import {
ListboxMode,
Expand Down Expand Up @@ -35,7 +35,7 @@ import {
} from '../../test-utils/interactions'
import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
import { Transition } from '../transition/transition'
import { Listbox } from './listbox'
import { Listbox, ListboxButton, ListboxOption, ListboxOptions } from './listbox'

jest.mock('../../hooks/use-id')

Expand Down Expand Up @@ -4811,3 +4811,44 @@ describe('Form compatibility', () => {
])
})
})

describe('transitions', () => {
it(
'should be possible to close the Listbox when using the `transition` prop',
suppressConsoleLogs(async () => {
render(
<Listbox>
<ListboxButton>Toggle</ListboxButton>
<ListboxOptions transition>
<ListboxOption value="alice">Alice</ListboxOption>
<ListboxOption value="bob">Bob</ListboxOption>
<ListboxOption value="charlie">Charlie</ListboxOption>
</ListboxOptions>
</Listbox>
)

// Focus the button
await focus(getListboxButton())

// Ensure the button is focused
assertActiveElement(getListboxButton())

// Open the listbox
await click(getListboxButton())

// Ensure the listbox is visible
assertListbox({ state: ListboxState.Visible })

// Close the listbox
await click(getListboxButton())

// Wait for the transition to finish, and the listbox to close
await waitFor(() => {
assertListbox({ state: ListboxState.InvisibleUnmounted })
})

// Ensure the button got the restored focus
assertActiveElement(getListboxButton())
})
)
})
11 changes: 9 additions & 2 deletions packages/@headlessui-react/src/components/listbox/listbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import React, {
useMemo,
useReducer,
useRef,
useState,
type CSSProperties,
type ElementType,
type MutableRefObject,
Expand Down Expand Up @@ -931,6 +932,7 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
...theirProps
} = props
let anchor = useResolvedAnchor(rawAnchor)
let [localOptionsElement, setLocalOptionsElement] = useState<HTMLElement | null>(null)

// Always enable `portal` functionality, when `anchor` is enabled
if (anchor) {
Expand All @@ -945,7 +947,7 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
let usesOpenClosedState = useOpenClosed()
let [visible, transitionData] = useTransition(
transition,
data.optionsElement,
localOptionsElement,
usesOpenClosedState !== null
? (usesOpenClosedState & State.Open) === State.Open
: data.listboxState === ListboxStates.Open
Expand Down Expand Up @@ -1023,7 +1025,12 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(

let [floatingRef, style] = useFloatingPanel(anchorOptions)
let getFloatingPanelProps = useFloatingPanelProps()
let optionsRef = useSyncRefs(ref, anchor ? floatingRef : null, actions.setOptionsElement)
let optionsRef = useSyncRefs(
ref,
anchor ? floatingRef : null,
actions.setOptionsElement,
setLocalOptionsElement
)

let searchDisposables = useDisposables()

Expand Down
45 changes: 43 additions & 2 deletions packages/@headlessui-react/src/components/menu/menu.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render } from '@testing-library/react'
import { render, waitFor } from '@testing-library/react'
import React, { createElement, useEffect } from 'react'
import {
MenuState,
Expand Down Expand Up @@ -31,7 +31,7 @@ import {
} from '../../test-utils/interactions'
import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
import { Transition } from '../transition/transition'
import { Menu } from './menu'
import { Menu, MenuButton, MenuItem, MenuItems } from './menu'

jest.mock('../../hooks/use-id')

Expand Down Expand Up @@ -3531,3 +3531,44 @@ describe('Mouse interactions', () => {
})
)
})

describe('transitions', () => {
it(
'should be possible to close the Menu when using the `transition` prop',
suppressConsoleLogs(async () => {
render(
<Menu>
<MenuButton>Toggle</MenuButton>
<MenuItems transition>
<MenuItem as="a">Alice</MenuItem>
<MenuItem as="a">Bob</MenuItem>
<MenuItem as="a">Charlie</MenuItem>
</MenuItems>
</Menu>
)

// Focus the button
await focus(getMenuButton())

// Ensure the button is focused
assertActiveElement(getMenuButton())

// Open the menu
await click(getMenuButton())

// Ensure the menu is visible
assertMenu({ state: MenuState.Visible })

// Close the menu
await click(getMenuButton())

// Wait for the transition to finish, and the menu to close
await waitFor(() => {
assertMenu({ state: MenuState.InvisibleUnmounted })
})

// Ensure the button got the restored focus
assertActiveElement(getMenuButton())
})
)
})
8 changes: 6 additions & 2 deletions packages/@headlessui-react/src/components/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import React, {
useMemo,
useReducer,
useRef,
useState,
type CSSProperties,
type Dispatch,
type ElementType,
Expand Down Expand Up @@ -620,10 +621,13 @@ function ItemsFn<TTag extends ElementType = typeof DEFAULT_ITEMS_TAG>(
let [state, dispatch] = useMenuContext('Menu.Items')
let [floatingRef, style] = useFloatingPanel(anchor)
let getFloatingPanelProps = useFloatingPanelProps()
let [localItemsElement, setLocalItemsElement] = useState<HTMLElement | null>(null)

let itemsRef = useSyncRefs(
ref,
anchor ? floatingRef : null,
useEvent((element) => dispatch({ type: ActionTypes.SetItemsElement, element }))
useEvent((element) => dispatch({ type: ActionTypes.SetItemsElement, element })),
setLocalItemsElement
)
let ownerDocument = useOwnerDocument(state.itemsElement)

Expand All @@ -635,7 +639,7 @@ function ItemsFn<TTag extends ElementType = typeof DEFAULT_ITEMS_TAG>(
let usesOpenClosedState = useOpenClosed()
let [visible, transitionData] = useTransition(
transition,
state.itemsElement,
localItemsElement,
usesOpenClosedState !== null
? (usesOpenClosedState & State.Open) === State.Open
: state.menuState === MenuStates.Open
Expand Down
Loading