-
Notifications
You must be signed in to change notification settings - Fork 383
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
Make custom elements w/ shadowRoot { display: contents } by default #723
Comments
What would worry me about this is that this isn't how any built-in element behaves. I don't think custom elements should be gratuitously different from built-ins.
|
Upon further thinking, there's 2 things that might make or break this idea. 1. It does not make sense if you're writing your custom elements in HTMLI can see what you're saying about how custom elements should not be gratuitously different from built-in elements. If you saw that HTML, you would expect it to behave the way that it currently does. 2. It really makes sense if you're creating components and their children all in JSFrom a react-like point of view, a component is just a set of predefined behaviour for a set of elements. The actual component has no affect on the box-model--only its contents do. While I agree that custom elements should behave like any other elements, my proposal is to affect custom elements that have a It's a double edged sword, in my opinion. Do you still manually write your HTML, or are you using a JS library that uses some sort of component architecture to define the makeup of your DOM? Maybe you don't see a huge difference, but I do. |
I don't think we should create asymmetries between those situations, even if other frameworks also use the word "component$ in such an asymmetrical way. |
I agree with all of your points, and this could be accomplished on the library level, but I still hold that it makes sense in the context of defining your components in JS. I'd love to hear what anyone else has to say on the matter as well before closing. |
Right now, there is no way to create a shadow root automatically via We certainly can't change the behavior of So I think this is a consideration for the future when we add a mechanism to automatically attach a shadow root when upgrading/instantiating a custom element. |
Have there been prior discussions about standardizing a way to automatically attach shadow roots to custom elements during their upgrade/instantiation? |
See #135 and related issues. |
It's plenty of easy to just write a class that automatically creates a shadow root for your instances. Making it default is not necessary; I've custom elements that don't need a shadow root, they only render themselves differently and distributing outer children to inner roots is not part of that custom rendering. A good example in the wild is A-Frame. Those elements don't use Shadow DOM at all, they are only a means to specify WebGL scenes (and so are my elements). But anyhow, React added "Fragments" to solve this problem, while Vue removed multi-root components which unfortunately introduces the problem into Vue. |
@ndugger: Is the "I" in that sentence the custom element author, or the custom element consumer, who should decide whether the custom element should simply act as a shell without its own box model? My first hunch is that it should be a characteristic of the custom element itself, meaning decided by the element author, not the element consumer. In which case, can't that be done via the |
@effulgentsia I agree with you. I was just finding it a little bit annoying to apply that to a lot of my components and saw this as an opportunity to improve upon that, but I can solve for it on the library level as well, so this is mostly moot. However, I think an alternative, like React's Fragments (see trusktr's comment), is worth exploring in the future. |
Yep! F.e. we can make it re-usable as a class factory mixin: function DisplayContents(BaseClass) {
BaseClass = BaseClass || HTMLElement
return class DisplayContents extends BaseClass {
constructor () {
super()
// apply the style somehow
this.style.display = 'contents'
}
}
} Then, if there's an existing custom element class, class CubeLayout extends HTMLElement { ... } it can be applied to the class: class CubeLayout extends DisplayContents(HTMLElement) { ... } The styling might be applied in a different way, but at least we can abstract it into a single place.
Hmmm. The thing that makes Fragments possible in React is that in React there is a whole separate tree from the DOM, and React's internal tree holds a reference to an actual fragment in this internal render tree. Then based on this internal tree it knows how to create a new tree (the DOM) where the new tree does not contain the Fragment in the structure. But Custom Elements, on the other hand, operate on a single tree (the DOM), so there isn't this opportunity to map one tree structure to another tree structure (that we've thought of yet at least). So far at least, There's the "composed tree" concept that the HTML engine maps to from the regular DOM, based on the composition of shadow roots, but outside of the HTML engine we don't really have access to that tree, we can only infer it from looking at open shadow roots (or if they are closed, monkey patching I'm sort of going off on a tangent now, but elements that are I've been thinking about this for a while, and have done some initial work over at infamous which can render to WebGL as well (not trying to advertise here, just like to share what I've been thinking around web components). I've put in place some features that allow it to track the composed tree, and soon this will map to the Three.js rendering so that making ShadowDOM-based Custom Elements will compose well together. It will give component authors the ability to compose and nest elements inside of each other to make these components highly-reusable. Imagine, for example, layouts designed for 3D space, that work based on distributing their content into slots. This is currently not possible with A-Frame (though you can just use React/Angular/Vue for that, but then you're sacrificing portability of you components by limiting them to a single framework). |
I think even then we shouldn't couple this as such side effects are often unexpected and would need to be configurable. At best we could consider some kind of toggle for this, but at that point it's probably better as a library feature or on top of adding default styles (whenever we added that). |
I agree this can be solved by #468 - the discussion is long to follow, basically with it you can give a stylesheet to a custom element, without attaching any shadow on it. |
Okay, let's close this in favor of that. If there's still desire for a specific thing around |
Late to the party but I'm concerned by the fact that having |
@Javarome don't worry. It's not possible to change the default. This issue is closed. |
No description provided. |
This idea is definitely jumping the gun a little bit, but...
As I've been working on a project that uses a react-like library that uses native web components, I've found that when I'm using them with flexbox, often times I just want the host custom element to simply act as a shell for the contents without having its own box model so that it doesn't hijack the flexbox context.
I did some searching today, and apparently
display: contents
does exactly this; it makes sense for this to be default behaviour, because if a custom element has ashadowRoot
, you generally expect the contents to define the makeup of the element.However, at the moment, in both Chrome and Firefox,
display: contents
is usable only behind a flag, and Edge probably doesn't even have it on their radar.Example:
In this example, you would have to reapply
display: flex
to thecustom-element
in order for theflex-item
s to actually be flex-items. However, ifcustom-element
's default wasdisplay: contents
, the flexbox context would pass through it, allowing theflex-item
s to actually be flex-items automatically.Make sense? Thoughts? I saw one closed issue that suggested them to be
display: block
by default, but this makes more sense, in my opinion.The text was updated successfully, but these errors were encountered: