From 3224a9e34dc76e9feb9d66c46aac1a87a756d616 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 26 Jun 2024 15:07:48 +0200 Subject: [PATCH] Fix incorrect `Transition` boundary for `Dialog` component (#3331) * always wrap `Dialog` in a `Transition` Initially we didn't do this, because it's a bit silly to do that if you already had a `Transition` component on the outside. E.g.: ```tsx setOpen(false)}> {/* ... */} ``` Because this means that we technically have this: ```tsx setOpen(false)}> {/* ... */} ``` The good part is that the inner `Transition` is rendering a `Fragment` and forwards all the props to the underlying element (the internal dialog). This way we have a guaranteed transition boundary. * use public `transition` API instead of private internal API This also mimics better what we are actually trying to do. * update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + .../src/components/dialog/dialog.test.tsx | 49 ++++++++++++++----- .../src/components/dialog/dialog.tsx | 3 +- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 757fa1b8d5..f8705e368c 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix issues spreading omitted props onto components ([#3313](https://github.com/tailwindlabs/headlessui/pull/3313)) - Fix initial `anchor="selection"` positioning ([#3324](https://github.com/tailwindlabs/headlessui/pull/3324)) - Fix render prop in `ComboboxOptions` to use `any` instead of `unknown` ([#3327](https://github.com/tailwindlabs/headlessui/pull/3327)) +- Fix incorrect `Transition` boundary for `Dialog` component ([#3331](https://github.com/tailwindlabs/headlessui/pull/3331)) ## [2.1.0] - 2024-06-21 diff --git a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx index 2967dde8f1..e9a71d4f1a 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx @@ -1,21 +1,20 @@ import { render } from '@testing-library/react' -import React, { createElement, Fragment, useCallback, useEffect, useRef, useState } from 'react' +import React, { Fragment, createElement, useCallback, useEffect, useRef, useState } from 'react' import { createPortal } from 'react-dom' -import { OpenClosedProvider, State } from '../../internal/open-closed' import { + DialogState, + PopoverState, assertActiveElement, assertDialog, assertDialogDescription, assertDialogTitle, assertPopoverPanel, - DialogState, getByText, getDialog, getDialogs, getPopoverButton, - PopoverState, } from '../../test-utils/accessibility-assertions' -import { click, focus, Keys, mouseDrag, press, shift } from '../../test-utils/interactions' +import { Keys, click, focus, mouseDrag, press, shift } from '../../test-utils/interactions' import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs' import type { PropsOf } from '../../types' import { Popover } from '../popover/popover' @@ -498,25 +497,51 @@ describe('Rendering', () => { it( 'should remove the scroll lock when the open closed state is `Closing`', suppressConsoleLogs(async () => { - function Example({ value = State.Open }) { + function Example({ open = true }) { + return ( + {}}> + + + + + ) + } + + let { rerender } = render() + + // The overflow should be there + expect(document.documentElement.style.overflow).toBe('hidden') + + // Re-render but with an exit transition + rerender() + + // The moment the dialog is closing, the overflow should be gone + expect(document.documentElement.style.overflow).toBe('') + }) + ) + + it( + 'should remove the scroll lock when the open closed state is `Closing` (using Transition wrapper)', + suppressConsoleLogs(async () => { + function Example({ open = true }) { return ( - - {}}> + + {}}> - + ) } - let { rerender } = render() + let { rerender } = render() // The overflow should be there expect(document.documentElement.style.overflow).toBe('hidden') - // Re-render but with the `Closing` state - rerender() + // Re-render but with an exit transition + rerender() // The moment the dialog is closing, the overflow should be gone expect(document.documentElement.style.overflow).toBe('') diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index 5db0b2ffaa..1a9b7fe6d8 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx @@ -390,8 +390,7 @@ function DialogFn( ) } - let inTransitionComponent = usesOpenClosedState !== null - if (!inTransitionComponent && open !== undefined && !rest.static) { + if ((open !== undefined || transition) && !rest.static) { return (