tailwindlabs / headlessui

Completely unstyled, fully accessible UI components, designed to integrate beautifully with Tailwind CSS.
https://headlessui.com
MIT License
25.99k stars 1.07k forks source link

headlessui-portal-root doesn't delete on modal close when using a Transition that doesn't fade #2114

Closed grenierdev closed 1 year ago

grenierdev commented 1 year ago

A minimal codesandbox that reproduces the bug described in https://github.com/tailwindlabs/headlessui/issues/1391 and https://github.com/tailwindlabs/headlessui/issues/1417, as requested by @adamwathan and @RobinMalfait.

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v1.7.7

What browser are you using?

Chrome

Reproduction URL

https://codesandbox.io/s/headlessui-dialog-bug-704xe7?file=/src/App.js

I copied and pasted the first Dialog example and changed the Transition of the backdrop.

Describe your issue

It appears that when using a backdrop with a Transition that doesn't fade from opacity-100 to opacity-0, the dialog still is somewhat present in the DOM. Since the backdrop is still in the DOM, the whole app is unusable.

In the codesandbox, after the dialog is closed, we can no longer click on the button that open's it since the backdrop is still there.

Hope it helps!

grenierdev commented 1 year ago

I was able to work around the bug by adding back the transition from opacity-100 to opacity-0 in addition to the blur.

RobinMalfait commented 1 year ago

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

The problem is that there is nothing to transition in your CodeSandbox, and you could safely remove the wrapping <Transition.Child />.

The reason for this is that you are using a very old version of Tailwind CSS (backdrop related classes don't exist in there) in that CodeSandbox link.

Instead of using:

<link
  rel="stylesheet"
  href="https://unpkg.com/@tailwindcss/ui/dist/tailwind-ui.min.css"
/>

I would either install the real Tailwind: https://tailwindcss.com/docs/installation Or use our CDN for reproductions: https://tailwindcss.com/docs/installation/play-cdn

<script src="https://cdn.tailwindcss.com"></script>

Switching to that CDN works out of the box:

Can you verify that this works for you? If not, can you create a minimal reproduction with your current setup?

grenierdev commented 1 year ago

@RobinMalfait thanks for your examination.

With the CDN script tag, it seems to fix the issue for the CodeSandbox.

I encountered the issue initially in my project and I'm not using TailwindCSS, but WindiCSS. The backdrop classes exist there.

Does it entail that HeadlessUI's components are tied-in to TailwindCSS in some ways?

RobinMalfait commented 1 year ago

@grenierdev it is not tied into Tailwind CSS no, the Transition component waits for a native transitionend event to happen, and I don't think that is being fired if you transition no backdrop to a backdrop or vice versa.

So I would either:

That said, I still want to fix the issue where it apparently triggers a transition but never finished (the "deadlock" kind of issue you are experiencing). But I hope you can continue with this information so far.

grenierdev commented 1 year ago

@RobinMalfait I'll try to provide you with minimal codesandbox with my current setup.

Thank you for your help!

NhanHo commented 1 year ago

@RobinMalfait Here is a minimal example showing the issue, packages are all latest version: https://codesandbox.io/p/sandbox/angry-sky-65zrps .

The transition on the backdrop prevents the portal from fully closing, removing that segment fixes the issue.

Edit: I will leave the comment here, but turned out the issue is that backdrop shouldn't be having the "hidden" class. Since this was copied from one of the component of tailwindui, it probably just got outdated.

tfilo commented 1 year ago

Hello, i use Transition to show/hide loader with overlay over the table. It works usually but from time to time, exactly this happens. Transition goes from opacity-100 to opacity-0 but after that it doesn’t remove itself from dom so transparent overlay stays over table and table it is not usable anymore. My setup is complex, i don’t know if i am able to create some minimal example with this issue. But it sounds like same issue mentiond in first comment.

my component looks like this, it is used as wrapper around my table.

<div className='relative'>
            <Transition
                show={loading}
                enter='transition-opacity duration-75'
                enterFrom='opacity-0'
                enterTo='opacity-100'
                leave='transition-opacity duration-150'
                leaveFrom='opacity-100'
                leaveTo='opacity-0'
            >
                <div
                    className='absolute left-0 right-0 top-0 bottom-0 z-10 flex justify-center bg-white opacity-75'
                    role='status'
                >
                    <div className='flex items-stretch self-center'>
                        <div className='h-12 w-12 animate-spin rounded-full border-5 border-grey-900 border-y-transparent'></div>
                    </div>
                    <span className='sr-only'>Loading...</span>
                </div>
            </Transition>
            {children}
        </div>
RobinMalfait commented 1 year ago

Thanks everyone! I could reproduce it and it should be solved. Thank you for your reproduction @NhanHo!

This should be fixed by #2318, and will be available in the next release.

You can already try it using: