Fix shadow-root bug closing Dialog containers
#2217
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Wow, thank you for digging so deep into this problem @RobinMalfait! |
|
You can already try it using |
|
Testing this locally now and it works great! Any ETA on when this will officially ship? |
|
I'll get a release out by the end of my day for you. Should be within ~2-3 hours. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We added better support for shadow roots being valid containers for the
Dialogcomponent in #2079. This means that if you have a 3rd party (or 1st party) tool that uses the shadow DOM, then theDialogused to close when interacting with those items.This was solved by @thecrypticace in the #2079 PR.
But I found that there is a bug here with one of these:
There is an issue with the Grammarly extension that injects a shadow root component inside the
Dialog.Panel. Yet, clicking on it closes theDialog. This issue occurs on Campsite for example (by @brianlovin).I can make it work by always allowing shadow roots with (before checking all the allowed containers):
The tree looks like this:
Looking at the
<grammarly-extensions>if I log$0.shadowRootI get theShadow Content (open)log.Looking at our check:
It results in
trueandfalse. The last value beingfalseis what breaks things and is the confusing part. It looks like the parent of the<grammarly-extension>is the<grammarly-mirror>and then immediately thehtmlSo we never saw the
Dialog.Panelelement and therefore closing theDialog...Time to dig deeper!
Well it turns out that the bug is very stupid actually and the solution is luckily very simple. Grammarly doesn't inject its container in the
<body>but as a sibling of<body>so we don't have any valid containers that contain this...So.. let's include
html > *except for thebodyandheaditself as valid containers and lo and behold:Fixes: #1832