-
-
Notifications
You must be signed in to change notification settings - Fork 423
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
feat: support attached shadow dom elements #618
Conversation
Original implementation did not properly handle nested shadow dom elements. Updated the traversal to manually step through the dom to ensure that all shadow dom ancestors are properly handled.
Hey @timmywil, I haven't really contributed to many open source projects before, so I apologize in advance if this is not the "right" way to do this - would you be able to take a look at this PR soon or provide a rough idea of when you'll be able to? I'm using the proposed fix in 2 other projects right now and would love to point to your repo instead of a local link or my fork. Let me know if there's anything else needed & thanks! |
Hey @timmywil, just wanted to ping again to see if you're able to review and merge or provide feedback. Thanks! |
@timmywil, can you please take a look at this and merge if acceptable (or provide guidance if you have any issues). I'd rather not permanently fork the project for this, but it's been quite a while with no movement. |
@timmywil any chance of this being merged? It all works for me. Thanks! |
Yes, this looks good. Sorry I haven't gotten to it sooner. |
Thanks!! A great library -- it just works and works well, compared to some others out there... |
@timmywil Thanks for this great little library. Would it be possible to get a new release anytime soon to include this PR? As-is the latest release is not usable by web components (without turning off shadow DOM usage entirely), and it looks like this is the only major change since the last release -- so may be an easy / low risk release. Thank you again. |
Hey @timmywil, any chance we could get a new release with this feature? It's been over two years since the last release and almost two years since this PR was merged. I'd love to go back to using the base library in my projects instead of my fork. |
PR Checklist
Please review the guidelines for contributing to this repository.
yarn test
against my changes and tests pass.test/unit/
or a new or altered demo indemo/
.yarn docs
), or no docs changes are needed.Description
Updates the
isAttached
helper to handle shadow dom elements.If the passed element's root node is a ShadowRoot, it setsparent
to the ShadowRoot's host and subsequently checks if the host is attached; otherwise it falls back to the pre-existing functionality.With additional testing, I found that the prior change was not sufficient to handle multiple nested shadow dom elements. The logic has been updated to manually traverse through the dom (handling both regular & shadow nodes) to ensure that any nesting is properly accounted for.
Fixes: #536