From 2ff8458b0043f900bf6f8267f01385a6f7df2f04 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 2 Sep 2024 00:17:28 +0800 Subject: [PATCH 1/3] Update CI workflows to use latest version of actions (#3445) GitHub began the Node16 deprecation process a year ago [1][2]. This commit updates CI workflows to use the latest Node20 actions. [1]: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/ [2]: https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/ Signed-off-by: Eng Zer Jun --- .github/workflows/main.yml | 22 +++++++++++----------- .github/workflows/prepare-release.yml | 6 +++--- .github/workflows/release-insiders.yml | 6 +++--- .github/workflows/release.yml | 6 +++--- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 491ec73b42..d325d3d03f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,12 +18,12 @@ jobs: steps: - name: Begin CI... - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Use Node ${{ env.NODE_VERSION }} - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: ${{ env.NODE_VERSION }} - - uses: actions/cache@v3 + - uses: actions/cache@v4 with: path: '**/node_modules' key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/package-lock.json') }} @@ -38,8 +38,8 @@ jobs: steps: - name: Begin CI... - uses: actions/checkout@v3 - - uses: actions/cache@v3 + uses: actions/checkout@v4 + - uses: actions/cache@v4 with: path: '**/node_modules' key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/package-lock.json') }} @@ -54,8 +54,8 @@ jobs: steps: - name: Begin CI... - uses: actions/checkout@v3 - - uses: actions/cache@v3 + uses: actions/checkout@v4 + - uses: actions/cache@v4 with: path: '**/node_modules' key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/package-lock.json') }} @@ -71,8 +71,8 @@ jobs: steps: - name: Begin CI... - uses: actions/checkout@v3 - - uses: actions/cache@v3 + uses: actions/checkout@v4 + - uses: actions/cache@v4 with: path: '**/node_modules' key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/package-lock.json') }} @@ -87,8 +87,8 @@ jobs: steps: - name: Begin CI... - uses: actions/checkout@v3 - - uses: actions/cache@v3 + uses: actions/checkout@v4 + - uses: actions/cache@v4 with: path: '**/node_modules' key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/package-lock.json') }} diff --git a/.github/workflows/prepare-release.yml b/.github/workflows/prepare-release.yml index 52ce28fc96..c8e80b6af1 100644 --- a/.github/workflows/prepare-release.yml +++ b/.github/workflows/prepare-release.yml @@ -24,7 +24,7 @@ jobs: node-version: [18] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - run: git fetch --tags -f @@ -41,13 +41,13 @@ jobs: echo "EOF" >> $GITHUB_ENV - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} registry-url: 'https://registry.npmjs.org' - name: Release - uses: softprops/action-gh-release@v1 + uses: softprops/action-gh-release@v2 with: draft: true tag_name: ${{ env.TAG_NAME }} diff --git a/.github/workflows/release-insiders.yml b/.github/workflows/release-insiders.yml index 310285f133..fcb7cf09f8 100644 --- a/.github/workflows/release-insiders.yml +++ b/.github/workflows/release-insiders.yml @@ -22,17 +22,17 @@ jobs: node-version: [20] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} registry-url: 'https://registry.npmjs.org' - name: Use cached node_modules id: cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: '**/node_modules' key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/package-lock.json') }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 36ecd254dc..f5f4ca51e9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -25,17 +25,17 @@ jobs: node-version: [18] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} registry-url: 'https://registry.npmjs.org' - name: Use cached node_modules id: cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: '**/node_modules' key: ${{ runner.os }}-${{ env.NODE_VERSION }}-modules-${{ hashFiles('**/package-lock.json') }} From 071aa0e260603997f21894c7781b6e7f6f21d996 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 3 Sep 2024 17:19:13 +0200 Subject: [PATCH 2/3] Fix components not properly closing when using the `transition` prop (#3448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR fixes a bug where the components don't always properly close when using the `transition` prop on those components. The issue here is that the internal `useTransition(…)` hook relies on a DOM node. Whenever the DOM node changes, we need to re-run the `useTransition(…)`. This is why we store the DOM element in state instead of relying on a `useRef(…)`. Let's say you have a `Popover` component, then the structure looks like this: ```ts Show Contents ``` We store a DOM reference to the button and the panel in state, and the state lives in the `Popover` component. The reason we do that is so that the button can reference the panel and the panel can reference the button. This is needed for some `aria-*` attributes for example: ```ts ``` For the transitions, we set some state to make sure that the panel is visible or hidden, then we wait for transitions to finish by listening to transition related events on the DOM node directly. If you now say, "hey panel, please re-render because you have to become visible/hidden" then the component re-renders, the panel DOM node (stored in the `Popover` component) eventually updates and then the `useTransition(…)` hooks receives the new value (either the DOM node or null when the leave transition is complete). The problem here is the round trip that it first has to go to the root `` component, re-render everything and provide the new DOM node to the `useTransition(…)` hook. The solution? Local state so that the panel can re-render on its own and doesn't require the round trip via the parent. Fixes: https://github.com/tailwindlabs/headlessui/issues/3438 Fixes: https://github.com/tailwindlabs/headlessui/issues/3437 Fixes: https://github.com/tailwindlabs/tailwindui-issues/issues/1625 --------- Co-authored-by: Jonathan Reinink --- packages/@headlessui-react/CHANGELOG.md | 4 +- .../src/components/combobox/combobox.test.tsx | 46 +++++++++++++++++- .../src/components/combobox/combobox.tsx | 16 ++++++- .../components/disclosure/disclosure.test.tsx | 47 +++++++++++++++++-- .../src/components/disclosure/disclosure.tsx | 12 ++++- .../src/components/listbox/listbox.test.tsx | 45 +++++++++++++++++- .../src/components/listbox/listbox.tsx | 16 ++++++- .../src/components/menu/menu.test.tsx | 45 +++++++++++++++++- .../src/components/menu/menu.tsx | 13 ++++- .../src/components/popover/popover.test.tsx | 39 ++++++++++++++- .../src/components/popover/popover.tsx | 28 ++++++++--- .../src/components/transition/transition.tsx | 6 +-- 12 files changed, 287 insertions(+), 30 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 89fb6bc26c..a8af42d98d 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -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 closing properly when using the `transition` prop ([#3448](https://github.com/tailwindlabs/headlessui/pull/3448)) ## [2.1.3] - 2024-08-23 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index 00fb4953ee..2eb66609a0 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -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, @@ -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 = () => {} @@ -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( + + Toggle + + + Alice + Bob + Charlie + + + ) + + // 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()) + }) + ) +}) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 131284aa31..8e582a0349 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -1672,14 +1672,26 @@ function OptionsFn( } let [floatingRef, style] = useFloatingPanel(anchor) + + // To improve the correctness of transitions (timing related race conditions), + // we track the element locally to this component, instead of relying on the + // context value. This way, the component can re-render independently of the + // parent component when the `useTransition(…)` hook performs a state change. + let [localOptionsElement, setLocalOptionsElement] = useState(null) + 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 diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx index 2c7aeb29df..d32844943c 100644 --- a/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx +++ b/packages/@headlessui-react/src/components/disclosure/disclosure.test.tsx @@ -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') @@ -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( + + Toggle + Contents + + ) + + // 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()) + }) + ) +}) diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx index 4a20bb2e5e..2f3e522627 100644 --- a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx +++ b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx @@ -11,6 +11,7 @@ import React, { useMemo, useReducer, useRef, + useState, type ContextType, type Dispatch, type ElementType, @@ -451,11 +452,18 @@ function PanelFn( let { close } = useDisclosureAPIContext('Disclosure.Panel') let mergeRefs = useMergeRefsFn() + // To improve the correctness of transitions (timing related race conditions), + // we track the element locally to this component, instead of relying on the + // context value. This way, the component can re-render independently of the + // parent component when the `useTransition(…)` hook performs a state change. + let [localPanelElement, setLocalPanelElement] = useState(null) + let panelRef = useSyncRefs( ref, useEvent((element) => { startTransition(() => dispatch({ type: ActionTypes.SetPanelElement, element })) - }) + }), + setLocalPanelElement ) useEffect(() => { @@ -468,7 +476,7 @@ function PanelFn( let usesOpenClosedState = useOpenClosed() let [visible, transitionData] = useTransition( transition, - state.panelElement, + localPanelElement, usesOpenClosedState !== null ? (usesOpenClosedState & State.Open) === State.Open : state.disclosureState === DisclosureStates.Open diff --git a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx index f603344b52..d75b01f5e3 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx @@ -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, @@ -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') @@ -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( + + Toggle + + Alice + Bob + Charlie + + + ) + + // 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()) + }) + ) +}) diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index e8c40e4365..e82cad3475 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -12,6 +12,7 @@ import React, { useMemo, useReducer, useRef, + useState, type CSSProperties, type ElementType, type MutableRefObject, @@ -932,6 +933,12 @@ function OptionsFn( } = props let anchor = useResolvedAnchor(rawAnchor) + // To improve the correctness of transitions (timing related race conditions), + // we track the element locally to this component, instead of relying on the + // context value. This way, the component can re-render independently of the + // parent component when the `useTransition(…)` hook performs a state change. + let [localOptionsElement, setLocalOptionsElement] = useState(null) + // Always enable `portal` functionality, when `anchor` is enabled if (anchor) { portal = true @@ -945,7 +952,7 @@ function OptionsFn( let usesOpenClosedState = useOpenClosed() let [visible, transitionData] = useTransition( transition, - data.optionsElement, + localOptionsElement, usesOpenClosedState !== null ? (usesOpenClosedState & State.Open) === State.Open : data.listboxState === ListboxStates.Open @@ -1023,7 +1030,12 @@ function OptionsFn( 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() diff --git a/packages/@headlessui-react/src/components/menu/menu.test.tsx b/packages/@headlessui-react/src/components/menu/menu.test.tsx index 0285959de9..a57e748f88 100644 --- a/packages/@headlessui-react/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.test.tsx @@ -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, @@ -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') @@ -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( + + Toggle + + Alice + Bob + Charlie + + + ) + + // 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()) + }) + ) +}) diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index 8da007529a..88b0074740 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -12,6 +12,7 @@ import React, { useMemo, useReducer, useRef, + useState, type CSSProperties, type Dispatch, type ElementType, @@ -620,10 +621,18 @@ function ItemsFn( let [state, dispatch] = useMenuContext('Menu.Items') let [floatingRef, style] = useFloatingPanel(anchor) let getFloatingPanelProps = useFloatingPanelProps() + + // To improve the correctness of transitions (timing related race conditions), + // we track the element locally to this component, instead of relying on the + // context value. This way, the component can re-render independently of the + // parent component when the `useTransition(…)` hook performs a state change. + let [localItemsElement, setLocalItemsElement] = useState(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) @@ -635,7 +644,7 @@ function ItemsFn( let usesOpenClosedState = useOpenClosed() let [visible, transitionData] = useTransition( transition, - state.itemsElement, + localItemsElement, usesOpenClosedState !== null ? (usesOpenClosedState & State.Open) === State.Open : state.menuState === MenuStates.Open diff --git a/packages/@headlessui-react/src/components/popover/popover.test.tsx b/packages/@headlessui-react/src/components/popover/popover.test.tsx index 85d00d0f71..86d9d44841 100644 --- a/packages/@headlessui-react/src/components/popover/popover.test.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.test.tsx @@ -1,4 +1,4 @@ -import { render } from '@testing-library/react' +import { render, waitFor } from '@testing-library/react' import React, { Fragment, act, createElement, useEffect, useRef, useState } from 'react' import ReactDOM from 'react-dom' import { @@ -2844,3 +2844,40 @@ describe('Nested popovers', () => { }) ) }) + +describe('transitions', () => { + it( + 'should be possible to close the Popover when using the `transition` prop', + suppressConsoleLogs(async () => { + render( + + Toggle + Contents + + ) + + // Focus the button + await focus(getPopoverButton()) + + // Ensure the button is focused + assertActiveElement(getPopoverButton()) + + // Open the popover + await click(getPopoverButton()) + + // Ensure the popover is visible + assertPopoverPanel({ state: PopoverState.Visible }) + + // Close the popover + await click(getPopoverButton()) + + // Wait for the transition to finish, and the popover to close + await waitFor(() => { + assertPopoverPanel({ state: PopoverState.InvisibleUnmounted }) + }) + + // Ensure the button got the restored focus + assertActiveElement(getPopoverButton()) + }) + ) +}) diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx index 018368d167..b1efa7eaf6 100644 --- a/packages/@headlessui-react/src/components/popover/popover.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.tsx @@ -763,13 +763,19 @@ function BackdropFn( ...theirProps } = props let [{ popoverState }, dispatch] = usePopoverContext('Popover.Backdrop') - let [backdropElement, setBackdropElement] = useState(null) - let backdropRef = useSyncRefs(ref, setBackdropElement) + + // To improve the correctness of transitions (timing related race conditions), + // we track the element locally to this component, instead of relying on the + // context value. This way, the component can re-render independently of the + // parent component when the `useTransition(…)` hook performs a state change. + let [localBackdropElement, setLocalBackdropElement] = useState(null) + + let backdropRef = useSyncRefs(ref, setLocalBackdropElement) let usesOpenClosedState = useOpenClosed() let [visible, transitionData] = useTransition( transition, - backdropElement, + localBackdropElement, usesOpenClosedState !== null ? (usesOpenClosedState & State.Open) === State.Open : popoverState === PopoverStates.Open @@ -865,11 +871,18 @@ function PanelFn( portal = true } + // To improve the correctness of transitions (timing related race conditions), + // we track the element locally to this component, instead of relying on the + // context value. This way, the component can re-render independently of the + // parent component when the `useTransition(…)` hook performs a state change. + let [localPanelElement, setLocalPanelElement] = useState(null) + let panelRef = useSyncRefs( internalPanelRef, ref, anchor ? floatingRef : null, - useEvent((panel) => dispatch({ type: ActionTypes.SetPanel, panel })) + useEvent((panel) => dispatch({ type: ActionTypes.SetPanel, panel })), + setLocalPanelElement ) let ownerDocument = useOwnerDocument(internalPanelRef) let mergeRefs = useMergeRefsFn() @@ -884,7 +897,7 @@ function PanelFn( let usesOpenClosedState = useOpenClosed() let [visible, transitionData] = useTransition( transition, - state.panel, + localPanelElement, usesOpenClosedState !== null ? (usesOpenClosedState & State.Open) === State.Open : state.popoverState === PopoverStates.Open @@ -1028,7 +1041,10 @@ function PanelFn( // Ignore sentinel buttons and items inside the panel for (let element of combined.slice()) { - if (element.dataset.headlessuiFocusGuard === 'true' || state.panel?.contains(element)) { + if ( + element.dataset.headlessuiFocusGuard === 'true' || + localPanelElement?.contains(element) + ) { let idx = combined.indexOf(element) if (idx !== -1) combined.splice(idx, 1) } diff --git a/packages/@headlessui-react/src/components/transition/transition.tsx b/packages/@headlessui-react/src/components/transition/transition.tsx index 31aaf07a4c..9f0922f0a3 100644 --- a/packages/@headlessui-react/src/components/transition/transition.tsx +++ b/packages/@headlessui-react/src/components/transition/transition.tsx @@ -319,12 +319,12 @@ function TransitionChildFn(null) + let [localContainerElement, setLocalContainerElement] = useState(null) let container = useRef(null) let requiresRef = shouldForwardRef(props) let transitionRef = useSyncRefs( - ...(requiresRef ? [container, ref, setContainerElement] : ref === null ? [] : [ref]) + ...(requiresRef ? [container, ref, setLocalContainerElement] : ref === null ? [] : [ref]) ) let strategy = theirProps.unmount ?? true ? RenderStrategy.Unmount : RenderStrategy.Hidden @@ -438,7 +438,7 @@ function TransitionChildFn` is done, but there is still a // child `` busy, then `visible` would be `false`, while // `state` would still be `TreeStates.Visible`. - let [, transitionData] = useTransition(enabled, containerElement, show, { start, end }) + let [, transitionData] = useTransition(enabled, localContainerElement, show, { start, end }) let ourProps = compact({ ref: transitionRef, From 75619eef3b21a0d0561b78e0f756e56e70ead31e Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 3 Sep 2024 17:23:03 +0200 Subject: [PATCH 3/3] 2.1.4 - @headlessui/react --- package-lock.json | 2 +- packages/@headlessui-react/CHANGELOG.md | 7 ++++++- packages/@headlessui-react/package.json | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 102011d77f..ddcd4f91ed 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11472,7 +11472,7 @@ }, "packages/@headlessui-react": { "name": "@headlessui/react", - "version": "2.1.1", + "version": "2.1.4", "license": "MIT", "dependencies": { "@floating-ui/react": "^0.26.16", diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index a8af42d98d..be3c86407a 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- Nothing yet! + +## [2.1.4] - 2024-09-03 + ### Fixed - Fix components not closing properly when using the `transition` prop ([#3448](https://github.com/tailwindlabs/headlessui/pull/3448)) @@ -754,7 +758,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Everything! -[unreleased]: https://github.com/tailwindlabs/headlessui/compare/@headlessui/react@v2.1.3...HEAD +[unreleased]: https://github.com/tailwindlabs/headlessui/compare/@headlessui/react@v2.1.4...HEAD +[2.1.4]: https://github.com/tailwindlabs/headlessui/compare/@headlessui/react@v2.1.3...@headlessui/react@v2.1.4 [2.1.3]: https://github.com/tailwindlabs/headlessui/compare/@headlessui/react@v2.1.2...@headlessui/react@v2.1.3 [2.1.2]: https://github.com/tailwindlabs/headlessui/compare/@headlessui/react@v2.1.1...@headlessui/react@v2.1.2 [2.1.1]: https://github.com/tailwindlabs/headlessui/compare/@headlessui/react@v2.1.0...@headlessui/react@v2.1.1 diff --git a/packages/@headlessui-react/package.json b/packages/@headlessui-react/package.json index 1af4880441..4e4203d39c 100644 --- a/packages/@headlessui-react/package.json +++ b/packages/@headlessui-react/package.json @@ -1,6 +1,6 @@ { "name": "@headlessui/react", - "version": "2.1.3", + "version": "2.1.4", "description": "A set of completely unstyled, fully accessible UI components for React, designed to integrate beautifully with Tailwind CSS.", "main": "dist/index.cjs", "typings": "dist/index.d.ts",