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

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

Closed
grenierdev opened this issue Dec 21, 2022 · 8 comments · Fixed by #2318
Assignees

Comments

@grenierdev
Copy link

A minimal codesandbox that reproduces the bug described in #1391 and #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
Copy link
Author

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 RobinMalfait self-assigned this Jan 4, 2023
@RobinMalfait
Copy link
Member

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
Copy link
Author

@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
Copy link
Member

@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:

  • Make sure you have some classes that are transitionable.
  • Drop the Transition.Child because it is not transitioning anything.

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
Copy link
Author

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

Thank you for your help!

@NhanHo
Copy link

NhanHo commented Jan 10, 2023

@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
Copy link

tfilo commented Feb 16, 2023

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
Copy link
Member

RobinMalfait commented Feb 28, 2023

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:

  • npm install @headlessui/react@insiders.

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

Successfully merging a pull request may close this issue.

4 participants