-
Notifications
You must be signed in to change notification settings - Fork 81
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
Observe shadowRoot mutations #18
base: main
Are you sure you want to change the base?
Conversation
3c0dba7
to
72d618f
Compare
@robdodson @alice FYI |
aedca2f
to
5834723
Compare
inert.js
Outdated
const rootObserver = new MutationObserver(boundOnMutation); | ||
const shadowRoot = this._rootElement.shadowRoot || this._rootElement.webkitShadowRoot; | ||
if (shadowRoot) { | ||
rootObserver.observe(shadowRoot, { childList: true, subtree: true }); |
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.
memo to self: we should be observing also the attributes..!
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.
@alice actually here we shoudn't observe attributes, as MutationObserver
will observe the ones of the target. In this case, the target is the shadowRoot which won't get any attribute change. this._observer
will take care of that.
cd883fb
to
40a9a32
Compare
test/index.js
Outdated
shadowButton.textContent = 'Shadow button'; | ||
host.shadowRoot.appendChild(shadowButton); | ||
// Give time to mutation observers. | ||
setTimeout(function() { |
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.
memo to self: use Promise instead of timeout
40a9a32
to
6e625aa
Compare
6e625aa
to
075a579
Compare
inert.js
Outdated
@@ -86,6 +120,9 @@ class InertRoot { | |||
this._observer.disconnect(); | |||
this._observer = null; | |||
|
|||
this._rootObserver.disconnect(); | |||
this._rootObserver = null; |
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.
Should we un-monkeypatch createShadowRoot
/attachShadow
here as well?
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.
Good point, will update!
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.
Done 👍
075a579
to
c600fc2
Compare
inert.js
Outdated
this._rootElement.removeAttribute('aria-hidden'); | ||
// Restore original attachShadow and createShadowRoot. | ||
delete this._rootElement.attachShadow; | ||
delete this._rootElement.createShadowRoot; |
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.
This assumes that no-one else has patched these methods
@alice @robdodson another approach could be to fire a |
23ac4f3
to
80e2528
Compare
PS: when I run
@robdodson any idea why? I tried to follow this guide but no success https://gist.github.com/DanHerbert/9520689 |
@valdrinkoshi which version of node and gulp are you running? |
|
hm that seems fine... Personally I use nvm to manage my node versions (https://github.com/creationix/nvm) and haven't had issues... |
@robdodson could you share your |
|
Yay! made it! Had to update the Concerning this PR, it's ready for review 👌 I ended up preferring two methods to patch/unpatch |
568bbfe
to
1c53778
Compare
@alice maybe we can spend some time reviewing this in Sydney? |
1c53778
to
48353e2
Compare
Fixes #17 by observing node changes on the
shadowRoot
of eachInertRoot
.If a node doesn't have yet a shadow root, we patch the methods to create a shadow root and we add our observer then! This might be slow, ideally we'd need to observe shadowRoot creation, but this is not possible yet in ShadowDOM v1 WICG/webcomponents#204