-
Notifications
You must be signed in to change notification settings - Fork 229
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
refactor: usage of root node for getting window or shadow root #1064
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow interesting find!
Looks great, thanks for testing and validating this better fix :)
@rebelchris I refactored it a bit to be safer from edge cases on any other browsers outside of Chromium, just to be sure. Let me know if have any concerns about it. Thank you for the awesome review! Also thoroughly tested it on our browser on all ends. |
Changes
Describe what this PR does
Selection
on shadow DOM, there seems to be no clear date of when to expect it.getRootNode
.Relevant links:
One example is if you visit our preview deployment, which is not whitelisted in our background worker, and if you inspect the HTML, you'll notice the shadow DOM to be present but the user mention still works since the
getRootNode
provides a more accurate for which the element was embedded.Preview deployment: https://preview.app.daily.dev/
Please make sure existing components are not breaking/affected by this PR
I have also manually tested the preview deployment on all major browsers we support (including Safari and Edge).
Manual Testing
On those affected packages:
Did you test the modified components media queries?
Did you test on actual mobile devices?
WT-104 #done