Add a @byContent selector for reparenting child content #5
Conversation
README.md
Outdated
| @@ -218,6 +219,44 @@ model ~ { | |||
| } | |||
| ``` | |||
|
|
|||
| #### byContent | |||
| This decorator allows you to capture the contents of a child of the element that is matched. | |||
| by a CSS selector. The can be used to reparent arbitrary child DOM content, which may not | |||
There was a problem hiding this comment.
Typo here, I think you meant then or the selector
lib/dom-model/DOMDecorators.js
Outdated
| import EmbedNode from './EmbedNode'; | ||
| import React from 'react'; | ||
|
|
||
| var _idCount = 0; |
There was a problem hiding this comment.
use let please. I will add a eslint config for this.
lib/dom-model/DOMDecorators.js
Outdated
| function findCommentNode(element, selector) { | ||
| for(let i = 0; i < element.childNodes.length; i++) { | ||
| let node = element.childNodes[i]; | ||
| if( node.nodeType === 8 |
There was a problem hiding this comment.
Theoretically we can use the constant Node.COMMENT_NODE to make the code clearer.
https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeType
Can you check browser support for that? At the very least you could add a comment to make it clear what 8 is
lib/dom-model/DOMNode.js
Outdated
| } | ||
| } | ||
|
|
||
| DOMNode.prototype.type = 'node'; |
There was a problem hiding this comment.
I don't think this is needed anymore.
| function findCommentNode(element, selector) { | ||
| for(let i = 0; i < element.childNodes.length; i++) { | ||
| let node = element.childNodes[i]; | ||
| if( node.nodeType === Node.COMMENT_NODE |
There was a problem hiding this comment.
I double checked this against the supported browsers.. The page was missing the info for Safari, Edge and IE. So, I checked all of those and Node.COMMENT_NODE was available.
All that to say, that it's ok to use the constant.
|
@alex-mirea I fixed up all of the the code review issues that you flagged. Thanks! |
Description
This is the implementation of a @byContent decorator that would allow the capturing of arbitrary DOM content that the web component user has placed into a wrapped web component.
Related Issue
#4
Motivation and Context
Adding this feature would allow us to turn a React container component into a web component. So, we could write containers such as popovers and dialogs in React and wrap them as web components. The user of the web components could pass in arbitrary content to be hosted in the container.
How Has This Been Tested?
I have developed a basic container control using this code and I have added a new test to the test suite to test it.
Screenshots (if appropriate):
Types of changes
Checklist: