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

Document overflow state not properly restored after opening successive dialogs #2168

Closed
ehwarren opened this issue Jan 9, 2023 · 5 comments · Fixed by #2215
Closed

Document overflow state not properly restored after opening successive dialogs #2168

ehwarren opened this issue Jan 9, 2023 · 5 comments · Fixed by #2215
Assignees

Comments

@ehwarren
Copy link

ehwarren commented Jan 9, 2023

What package within Headless UI are you using?
@headlessui/react

What version of that package are you using?
"^1.7.4"

What browser are you using?
Microsoft Edge - Version 108.0.1462.76 (Official build) (64-bit)

Reproduction URL
https://double-modal-all-across-the-sky.vercel.app/
https://github.com/ehwarren/double-modal-all-across-the-sky

Describe your issue
Same issue as described in #881
Also previously described in #1000

In summary:

When one modal is opened as a previous one is closing, the incorrect document.documentElement.style state is being stored. This causes the overflow to still be hidden when the second dialog is closed.

Steps to reproduce behavior:

Load page - height is set to 200vh, so scroll bar always present.
Click menu hamburger (top right). note scroll is not present, due to scroll lock, as expected
Click any "nav item" in the menu dialog that has opened. Note the new alert dialog appears, BUT, document overflow hidden has been removed
Close the alert dialog Document overflow has been reverted back to overflow hidden, and the page is no longer scrollable

@ehwarren
Copy link
Author

ehwarren commented Jan 10, 2023

Realizing after more digging this is officially not supported, with some "workarounds" listed here. #1199
#1744

I will leave this issue open and let you decide on what to do, but perhaps even a warning on docs stating that this is not supported would be a nice idea.

@RobinMalfait
Copy link
Member

RobinMalfait commented Jan 20, 2023

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

This is indeed currently something we don't support. Due to the Transition component it happens that multiple Dialogs are open at the same time.

@thecrypticace was working on a fix for this and if it is doable to implement properly then we will add it. I'll let him decide on this one :)

@thecrypticace
Copy link
Contributor

Hey! This should be fixed (via #2215), and will be available in the next release.

You can try it already by using our insiders build (may take a couple of minutes to publish):

  • npm install @headlessui/react@insiders
  • npm install @headlessui/vue@insiders

@michaelmerrill
Copy link

Quick test locally and it solves the issue for me. Thanks @thecrypticace!

@jeff-bacon
Copy link

jeff-bacon commented Feb 19, 2024

The insider build does not fix this issue for me. I have two dialogs, clicking a button in the first will close the first, and open the second. The second dialog cannot scroll on my mobile device. I've tested on iphone 15 using Microsoft Edge as well as Firefox and get the same results on both. The scrolling works fine on desktop in all situations. The scrolling also works fine if I only open the second dialog without triggering it from the first.

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

Successfully merging a pull request may close this issue.

5 participants