Add support for wrapping elm rendered dom in shadow dom #17
Conversation
src/index.js
Outdated
| setupPorts(elmElement.ports) | ||
| } else if (elmVersion === '0.18') { | ||
| const elmElement = ElmComponent.embed(this, flags) | ||
| const elmElement = ElmComponent.embed(elmDiv, flags) |
There was a problem hiding this comment.
Should this not be parentDiv? Or if Elm rendering on 0.18 can't work, we should log a warning here if the flag was active
There was a problem hiding this comment.
That's probably a bug, good catch. I'll update the PR but I don't have time to test the change right now. I'll try it out during the week.
There was a problem hiding this comment.
I confirm that it was a bug and that the last change fixed it.
src/index.js
Outdated
| context.flags = flags | ||
|
|
||
| var elmDiv = this; | ||
| var parentDiv = this; |
There was a problem hiding this comment.
We should use const/let rather than var here.
const parentDiv = useShadowDom ? this.attachShadow({ mode: 'open' }) : this;
maybe?
There was a problem hiding this comment.
Indeed that's cleaner. And combined with your previous comment, I can use `const elmDiv in the 0.19 branch.
|
@mdevlamynck thank you for this! I'll merge now and we'll post a new version shortly |
|
I a bit late to the party 🎉. (please don't get mad at me for commenting on a closed MR) |
|
It helps isolate the style of the component from the style of the page so that the rendering of component is not affected by the page embedding it. The idea is to ease the building of reusable component independently of where there are going to be used. |
This adds the boolean option
useShadowDom(defaults to false to avoid breaking changes) to theregister()function. When enabled it wraps the div elm renders to in a shadow dom. This also adds a section in the README for documenting the feature.