Skip to content

Commit adfe121

Browse files
authored
Start cleanup phase of the Dialog component when going into the Closing state (#2264)
* introduce `opening` and `closing` states Also represent them as bits so that we can easily combine them while we are transitioning from one state to the other. * update `open/closed` state checks Instead of checking whether it is in one state or an other, we can check if the current state contains some potential sub-state. This allows us to still check if we are in the `Open` state, while also `Closing` because the state will be `S.Open | S.Closing`. * expose `flags` from the `useFlags` hook * add the `Closing` and `Opening` states to the Open/Closed state * create dedicated `abcEnabled` variables * keep the `State.Closing` into account for `scroll locking` and `inert others` * add a test for the `Closing` state impacting the `Dialog` component * cleanup unused imports * add `unmount` util to the Vue Test renderer * update changelog
1 parent 22e388e commit adfe121

File tree

22 files changed

+264
-75
lines changed

22 files changed

+264
-75
lines changed

packages/@headlessui-react/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414
- Re-focus `Combobox.Input` when a `Combobox.Option` is selected ([#2272](https://github.com/tailwindlabs/headlessui/pull/2272))
1515
- Ensure we reset the `activeOptionIndex` if the active option is unmounted ([#2274](https://github.com/tailwindlabs/headlessui/pull/2274))
1616
- Improve Ref type for forwarded `Switch`'s ref ([#2277](https://github.com/tailwindlabs/headlessui/pull/2277))
17+
- Start cleanup phase of the Dialog component when going into the Closing state ([#2264](https://github.com/tailwindlabs/headlessui/pull/2264))
1718

1819
## [1.7.10] - 2023-02-06
1920

packages/@headlessui-react/src/components/combobox/combobox.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1174,7 +1174,7 @@ let Options = forwardRefWithAs(function Options<
11741174
let usesOpenClosedState = useOpenClosed()
11751175
let visible = (() => {
11761176
if (usesOpenClosedState !== null) {
1177-
return usesOpenClosedState === State.Open
1177+
return (usesOpenClosedState & State.Open) === State.Open
11781178
}
11791179

11801180
return data.comboboxState === ComboboxState.Open

packages/@headlessui-react/src/components/dialog/dialog.test.tsx

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { createPortal } from 'react-dom'
12
import React, { createElement, useRef, useState, Fragment, useEffect, useCallback } from 'react'
23
import { render } from '@testing-library/react'
34

@@ -24,7 +25,7 @@ import {
2425
import { click, mouseDrag, press, Keys, shift } from '../../test-utils/interactions'
2526
import { PropsOf } from '../../types'
2627
import { Transition } from '../transitions/transition'
27-
import { createPortal } from 'react-dom'
28+
import { OpenClosedProvider, State } from '../../internal/open-closed'
2829

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

@@ -411,6 +412,34 @@ describe('Rendering', () => {
411412
expect(document.documentElement.style.overflow).toBe('')
412413
})
413414
)
415+
416+
it(
417+
'should remove the scroll lock when the open closed state is `Closing`',
418+
suppressConsoleLogs(async () => {
419+
function Example({ value = State.Open }) {
420+
return (
421+
<OpenClosedProvider value={value}>
422+
<Dialog open={true} onClose={() => {}}>
423+
<input id="a" type="text" />
424+
<input id="b" type="text" />
425+
<input id="c" type="text" />
426+
</Dialog>
427+
</OpenClosedProvider>
428+
)
429+
}
430+
431+
let { rerender } = render(<Example value={State.Open} />)
432+
433+
// The overflow should be there
434+
expect(document.documentElement.style.overflow).toBe('hidden')
435+
436+
// Re-render but with the `Closing` state
437+
rerender(<Example value={State.Open | State.Closing} />)
438+
439+
// The moment the dialog is closing, the overflow should be gone
440+
expect(document.documentElement.style.overflow).toBe('')
441+
})
442+
)
414443
})
415444

416445
describe('Dialog.Overlay', () => {

packages/@headlessui-react/src/components/dialog/dialog.tsx

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,7 @@ let DialogRoot = forwardRefWithAs(function Dialog<
140140
let usesOpenClosedState = useOpenClosed()
141141
if (open === undefined && usesOpenClosedState !== null) {
142142
// Update the `open` prop based on the open closed state
143-
open = match(usesOpenClosedState, {
144-
[State.Open]: true,
145-
[State.Closed]: false,
146-
})
143+
open = (usesOpenClosedState & State.Open) === State.Open
147144
}
148145

149146
let containers = useRef<Set<MutableRefObject<HTMLElement | null>>>(new Set())
@@ -209,8 +206,21 @@ let DialogRoot = forwardRefWithAs(function Dialog<
209206
// in between. We only care abou whether you are the top most one or not.
210207
let position = !hasNestedDialogs ? 'leaf' : 'parent'
211208

209+
// When the `Dialog` is wrapped in a `Transition` (or another Headless UI component that exposes
210+
// the OpenClosed state) then we get some information via context about its state. When the
211+
// `Transition` is about to close, then the `State.Closing` state will be exposed. This allows us
212+
// to enable/disable certain functionality in the `Dialog` upfront instead of waiting until the
213+
// `Transition` is done transitioning.
214+
let isClosing =
215+
usesOpenClosedState !== null ? (usesOpenClosedState & State.Closing) === State.Closing : false
216+
212217
// Ensure other elements can't be interacted with
213-
useInertOthers(internalDialogRef, hasNestedDialogs ? enabled : false)
218+
let inertOthersEnabled = (() => {
219+
if (!hasNestedDialogs) return false
220+
if (isClosing) return false
221+
return enabled
222+
})()
223+
useInertOthers(internalDialogRef, inertOthersEnabled)
214224

215225
let resolveContainers = useEvent(() => {
216226
// Third party roots
@@ -229,25 +239,36 @@ let DialogRoot = forwardRefWithAs(function Dialog<
229239
})
230240

231241
// Close Dialog on outside click
232-
useOutsideClick(() => resolveContainers(), close, enabled && !hasNestedDialogs)
242+
let outsideClickEnabled = (() => {
243+
if (!enabled) return false
244+
if (hasNestedDialogs) return false
245+
return true
246+
})()
247+
useOutsideClick(() => resolveContainers(), close, outsideClickEnabled)
233248

234249
// Handle `Escape` to close
250+
let escapeToCloseEnabled = (() => {
251+
if (hasNestedDialogs) return false
252+
if (dialogState !== DialogStates.Open) return false
253+
return true
254+
})()
235255
useEventListener(ownerDocument?.defaultView, 'keydown', (event) => {
256+
if (!escapeToCloseEnabled) return
236257
if (event.defaultPrevented) return
237258
if (event.key !== Keys.Escape) return
238-
if (dialogState !== DialogStates.Open) return
239-
if (hasNestedDialogs) return
240259
event.preventDefault()
241260
event.stopPropagation()
242261
close()
243262
})
244263

245264
// Scroll lock
246-
useScrollLock(
247-
ownerDocument,
248-
dialogState === DialogStates.Open && !hasParentDialog,
249-
resolveContainers
250-
)
265+
let scrollLockEnabled = (() => {
266+
if (isClosing) return false
267+
if (dialogState !== DialogStates.Open) return false
268+
if (hasParentDialog) return false
269+
return true
270+
})()
271+
useScrollLock(ownerDocument, scrollLockEnabled, resolveContainers)
251272

252273
// Trigger close when the FocusTrap gets hidden
253274
useEffect(() => {

packages/@headlessui-react/src/components/disclosure/disclosure.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE
375375
let usesOpenClosedState = useOpenClosed()
376376
let visible = (() => {
377377
if (usesOpenClosedState !== null) {
378-
return usesOpenClosedState === State.Open
378+
return (usesOpenClosedState & State.Open) === State.Open
379379
}
380380

381381
return state.disclosureState === DisclosureStates.Open

packages/@headlessui-react/src/components/listbox/listbox.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -756,7 +756,7 @@ let Options = forwardRefWithAs(function Options<
756756
let usesOpenClosedState = useOpenClosed()
757757
let visible = (() => {
758758
if (usesOpenClosedState !== null) {
759-
return usesOpenClosedState === State.Open
759+
return (usesOpenClosedState & State.Open) === State.Open
760760
}
761761

762762
return data.listboxState === ListboxStates.Open

packages/@headlessui-react/src/components/menu/menu.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ let Items = forwardRefWithAs(function Items<TTag extends ElementType = typeof DE
419419
let usesOpenClosedState = useOpenClosed()
420420
let visible = (() => {
421421
if (usesOpenClosedState !== null) {
422-
return usesOpenClosedState === State.Open
422+
return (usesOpenClosedState & State.Open) === State.Open
423423
}
424424

425425
return state.menuState === MenuStates.Open

packages/@headlessui-react/src/components/popover/popover.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ let Overlay = forwardRefWithAs(function Overlay<
616616
let usesOpenClosedState = useOpenClosed()
617617
let visible = (() => {
618618
if (usesOpenClosedState !== null) {
619-
return usesOpenClosedState === State.Open
619+
return (usesOpenClosedState & State.Open) === State.Open
620620
}
621621

622622
return popoverState === PopoverStates.Open
@@ -693,7 +693,7 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE
693693
let usesOpenClosedState = useOpenClosed()
694694
let visible = (() => {
695695
if (usesOpenClosedState !== null) {
696-
return usesOpenClosedState === State.Open
696+
return (usesOpenClosedState & State.Open) === State.Open
697697
}
698698

699699
return state.popoverState === PopoverStates.Open

packages/@headlessui-react/src/components/transitions/transition.tsx

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { useEvent } from '../../hooks/use-event'
3232
import { useDisposables } from '../../hooks/use-disposables'
3333
import { classNames } from '../../utils/class-names'
3434
import { env } from '../../utils/env'
35+
import { useFlags } from '../../hooks/use-flags'
3536

3637
type ContainerElement = MutableRefObject<HTMLElement | null>
3738

@@ -359,18 +360,32 @@ let TransitionChild = forwardRefWithAs(function TransitionChild<
359360
return show ? 'enter' : 'leave'
360361
})() as TransitionDirection
361362

363+
let transitionStateFlags = useFlags(0)
364+
362365
let beforeEvent = useEvent((direction: TransitionDirection) => {
363366
return match(direction, {
364-
enter: () => events.current.beforeEnter(),
365-
leave: () => events.current.beforeLeave(),
367+
enter: () => {
368+
transitionStateFlags.addFlag(State.Opening)
369+
events.current.beforeEnter()
370+
},
371+
leave: () => {
372+
transitionStateFlags.addFlag(State.Closing)
373+
events.current.beforeLeave()
374+
},
366375
idle: () => {},
367376
})
368377
})
369378

370379
let afterEvent = useEvent((direction: TransitionDirection) => {
371380
return match(direction, {
372-
enter: () => events.current.afterEnter(),
373-
leave: () => events.current.afterLeave(),
381+
enter: () => {
382+
transitionStateFlags.removeFlag(State.Opening)
383+
events.current.afterEnter()
384+
},
385+
leave: () => {
386+
transitionStateFlags.removeFlag(State.Closing)
387+
events.current.afterLeave()
388+
},
374389
idle: () => {},
375390
})
376391
})
@@ -425,10 +440,12 @@ let TransitionChild = forwardRefWithAs(function TransitionChild<
425440
return (
426441
<NestingContext.Provider value={nesting}>
427442
<OpenClosedProvider
428-
value={match(state, {
429-
[TreeStates.Visible]: State.Open,
430-
[TreeStates.Hidden]: State.Closed,
431-
})}
443+
value={
444+
match(state, {
445+
[TreeStates.Visible]: State.Open,
446+
[TreeStates.Hidden]: State.Closed,
447+
}) | transitionStateFlags.flags
448+
}
432449
>
433450
{render({
434451
ourProps,
@@ -457,10 +474,7 @@ let TransitionRoot = forwardRefWithAs(function Transition<
457474
let usesOpenClosedState = useOpenClosed()
458475

459476
if (show === undefined && usesOpenClosedState !== null) {
460-
show = match(usesOpenClosedState, {
461-
[State.Open]: true,
462-
[State.Closed]: false,
463-
})
477+
show = (usesOpenClosedState & State.Open) === State.Open
464478
}
465479

466480
if (![true, false].includes(show as unknown as boolean)) {

packages/@headlessui-react/src/hooks/use-flags.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ export function useFlags(initialFlags = 0) {
88
let removeFlag = useCallback((flag: number) => setFlags((flags) => flags & ~flag), [setFlags])
99
let toggleFlag = useCallback((flag: number) => setFlags((flags) => flags ^ flag), [setFlags])
1010

11-
return { addFlag, hasFlag, removeFlag, toggleFlag }
11+
return { flags, addFlag, hasFlag, removeFlag, toggleFlag }
1212
}

0 commit comments

Comments
 (0)