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

Transition does not call afterLeave when using nested Transition.Child #1866

Closed
dcastil opened this issue Sep 19, 2022 · 5 comments
Closed

Comments

@dcastil
Copy link
Contributor

dcastil commented Sep 19, 2022

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v1.7.2

What browser are you using?

Chrome

Reproduction URL

https://codesandbox.io/s/headlessui-transition-bug-afterleave-3k435m?file=/src/App.tsx:666-1441

  1. Open link above
  2. Open console of Code Sandbox
  3. Click on the "Click to transition" button
  4. Observe that console.log('unmount') in afterLeave callback never gets called.

Describe your issue

When using a nested Transition.Child within a Transition component, the afterLeave callback from the Transition component doesn't get called when the exit transition ends.

Related: #1364

@RobinMalfait
Copy link
Collaborator

Hey! Thank you for your bug report!
Much appreciated! 🙏

I see what you are saying, but what is your use case for using Transition.Child components without transition classes on them? We will probably still fix it but it feels a bit odd to do this in the first place.

I mean, if you buy a car and take out the engine, it won't work as expected either right 😅

@dcastil
Copy link
Contributor Author

dcastil commented Sep 20, 2022

Ah yeah sorry the Transition.Child without transition classes was a lazy mistake of mine. I just tried to create a reproducible example that is realistic but so far I couldn't reproduce the issue. I'll comment here once I'm able to create a minimal reproduction example.

In the actual code I have in a production app there is a Dialog component which is using the Radix UI dialog primitive together with the Headless UI Transition component to animate its presence. The problem there is that afterLeave on the Transition component doesn't get called when hasTransparentOverlay is falsy and therefore the Transition doesn't have an animation of its own, but Transition.Child still has animation properties.

I think it is a race condition of some sorts because I'm not able to reproduce the problem in CodeSandbox but in the app I'm working on it happens 100% of the time.

Note the comments in the Transition.Child component, there is another issue with the enter animation not working if I put the transition-related classes into the enter prop. I probably have to create a separate issue for that at some point. This issue only appears with larger render trees (not huge but larger than a simple example), e.g. when I replace the dialog content with a <button>Hello</button>, the issue with enter disappears.

Actual code
import { Transition } from '@headlessui/react'
import {
    Root as DialogRoot,
    DialogPortal,
    DialogOverlay,
    DialogContent,
} from '@radix-ui/react-dialog'
import { Fragment } from 'react'

// … more imports and not so important code

interface DialogProps {
    appear?: boolean
    isOpen: boolean
    hasVariableHeight?: boolean
    hasTransparentOverlay?: boolean
    children: React.ReactNode
    close(): void
    onUnmount?(): void
}

export function Dialog({
    appear,
    isOpen,
    hasVariableHeight,
    hasTransparentOverlay,
    children,
    close,
    onUnmount,
}: DialogProps) {
    return (
        <DialogRoot open={isOpen} onOpenChange={(isOpen) => !isOpen && close()}>
            <DialogPortal forceMount>
                <Transition
                    as={Fragment}
                    appear={appear}
                    show={isOpen}
                    {...(!hasTransparentOverlay && {
                        enter: 'transition-colors ease-out duration-300',
                        enterFrom: 'bg-transparent',
                        enterTo: 'bg-black/25',
                        leave: 'transition-colors ease-in duration-150',
                        leaveFrom: 'bg-black/25',
                        leaveTo: 'bg-transparent',
                    })}
                >
                    <DialogOverlay className="fixed inset-0 overflow-y-scroll" forceMount>
                        <VerticalPositionContainer hasVariableHeight={hasVariableHeight}>
                            <Transition.Child
                                as={Fragment}
                                enterFrom="opacity-0 scale-95"
                                // Putting transition properties in here because large render trees like the command line cause race conditions in which animations from classes put in `enter` won't work.
                                enterTo="transition ease-out duration-150 opacity-100 scale-100"
                                leave="transition ease-in duration-75"
                                leaveFrom="opacity-100 scale-100"
                                leaveTo="opacity-0 scale-95"
                                // To counter transition class staying in Transition component when entered
                                entered="transition-none"
                                // Calling onUnmount here instead of parent <Transition /> because of https://github.com/tailwindlabs/headlessui/issues/1866
                                afterLeave={onUnmount}
                            >
                                <DialogContent asChild forceMount>
                                    <KeyboardShortcutProvider
                                        // !pointer-events-auto: Prevent Radix UI disabling pointer events when there are modal elements open inside. We render an overlay on top while those modal elements are open, but we want to enable pointer events during the exit animation of modal elements.
                                        className="outline-none focus-visible:ring !pointer-events-auto"
                                    >
                                        {/* Necessary because KeyboardShortcutProvider stops propagation which makes Radix UI's internal escape handler not work anymore */}
                                        <UseEscapeStack callback={close} />
                                        {children}
                                    </KeyboardShortcutProvider>
                                </DialogContent>
                            </Transition.Child>
                        </VerticalPositionContainer>
                    </DialogOverlay>
                </Transition>
            </DialogPortal>
        </DialogRoot>
    )
}

@RobinMalfait
Copy link
Collaborator

Hmmm yeah if you could figure out a minimal reproduction that would be awesome. That said, our components communicate with each other so the Transition component would receive a show prop, the Dialog component from Headless UI would then internally receive the open state from the Transition component so that the timing is right. The open data would only change to false the moment the transition is complete.

I can see a potential issue where in this code:

<DialogRoot open={isOpen} onOpenChange={(isOpen) => !isOpen && close()}>

The onOpenChange gets called before the transition is complete. If that's the case, then I'm assuming that the Dialog from Radix UI gets unmounted including its children and therefore the Transition component from Headless UI gets unmounted. If that is the case in your application as well, then I'm afraid that we can't do anything about that since the control of that state is out of our hands.

One thing we do showcase in our docs is that the Transition component is on the outside, so that it can control its children and not the other way around.

https://headlessui.com/react/dialog#transitions

@dcastil
Copy link
Contributor Author

dcastil commented Sep 21, 2022

The open prop of the DialogRoot controls its focus trap, scroll lock, etc. which is why it's in sync with the show prop of the Transition component. All those things should get disabled at the moment the exit transition starts. It doesn't unmount any components because of the forceMount prop on all the Radix components (that's the intended way to use Radix with close animations).

I'll try to put the Transition on the outside and report back.

@RobinMalfait
Copy link
Collaborator

Hey!

Going to close this one for now. If you can reproduce the issue with the Transition on the outside then feel free to open a new issue with a reproduction repo / CodeSandbox attached. We can also re-open this issue, but interactions on older PRs/issues tend to get lost quicker.

I hope you figured it out for your use case 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants