Skip to content
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

[Shadow] Slotting indirect children #574

Closed
ghost opened this issue Sep 23, 2016 · 18 comments
Closed

[Shadow] Slotting indirect children #574

ghost opened this issue Sep 23, 2016 · 18 comments

Comments

@ghost
Copy link

ghost commented Sep 23, 2016

I am trying to implement a tabs custom element and had to realise that it is probably not possible to implement what I want to do. Let's start with the markup I would like to use:

<my-tabs>
    <my-tab>
        <my-tab-title>
            Title 1
        </my-tab-title>
        <my-tab-content>
            Content 1
        </my-tab-content>
    </my-tab>
    <my-tab>
        <my-tab-title>
            Title 2
        </my-tab-title>
        <my-tab-content>
            Content 2
        </my-tab-content>
    </my-tab>
</my-tabs>

The composed DOM should look like this:

<my-tabs>
    <ul>
        <li>Title 1</li>
        <li>Title 2</li>
    </ul>
    <article>
        Content 1
    </article>
    <article hidden>
        Content 2
    </article>
</my-tabs>

(The titles and contents could still be wrapped with their custom element.)

My first attempt was to attach a shadow to my-tabs and use a named slot for the titles and contents:

shadowRoot.innerHTML = `
<slot name="title"><slot>
<slot name="content"><slot>
`;

The markup would then have been:

<my-tabs>
    <my-tab>
        <my-tab-title slot="title">
            Title 1
        </my-tab-title>
        <my-tab-content slot="content">
            Content 1
        </my-tab-content>
    </my-tab>
    ...
</my-tabs>

This didn't work because it seems that a slot can only render elements which are a direct child of the shadow holder element. I was already surprised about the fact that I had to set the slot attribute (#343), but I thought that I could still work around this by setting the slot attributes with a MutationObserver if I really wanted to. This, however, seems to be a limitation which cannot be worked around.

Next, I tried to implement the tabs component without using any slots by coping/cloning the parts I need in a MutationObserver. I quickly had to realise that this solution is simply not good enough because if the titles/contents contain a stateful element, I cannot just copy the HTML/clone the DOM if the user interacts with the stateful element and changes its internal state.

Unfortunately, it seems like there is not even an imperative API which would allow me to set the slot of an element manually apart from setting the slot attribute. My hope was that assignedSlot might be writeable which would have allowed me to set the slot of the titles/contents without having this direct child limitation. Well, it's readonly.

Did I miss anything?

What is the reason that you cannot slot an indirect child? Is it performance? If the slot attribute is set and the shadow does not contain a slot with this name, couldn't it just check the next parent until it finds a slot with this name?

@rniwa
Copy link
Collaborator

rniwa commented Sep 23, 2016

This was done for both performance and semantics reasons. Since each ancestor of a node can have its own shadow tree, we must define the shadow tree to which a node is assigned, and the latter turned out be tricky. At the end, WebKit team at Apple has argued for supporting this feature but we couldn't reach a consensus to do so in v1.

@ghost
Copy link
Author

ghost commented Sep 23, 2016

Is there any chance that this could be changed for v1?

The problem with limitation is that there is no acceptable workaround. The only working (but very bad) solution would be to implement something which is very similar to a shadow dom polyfill.

Why is it so tricky to find the shadow to which a node is assigned? I would have suggested the following algorithm:

  1. If there is no slot attribute, the node is assigned to the shadow of the direct parent element (if present).
  2. If there is a slot attribute set, the node is assigned to the shadow of the direct parent element if present and if it has a slot with this name.
  3. If there is a slot attribute set and the direct parent does not have a shadow or the shadow does not have a slot with this name, then it is assigned to the closest shadow with a slot with this name.

For most of the nodes 1. or 2. applies. These nodes will not be slowed down at all. For the remaining other nodes, it will have to check a few additional ancestors.

Do you remember the exact reasons why it didn't make it into the spec?

@rniwa
Copy link
Collaborator

rniwa commented Sep 23, 2016

@trusktr
Copy link

trusktr commented Sep 27, 2016

Hey guys, what do you think of something like #576? It is more explicit, although it would also mean that some element could arbitrarily throw content into some other element's shadow root.

Is that okay? Or does limiting it to shadow root of an ancestor make more sense?

@ghost
Copy link
Author

ghost commented Sep 28, 2016

@trusktr my tabs example would then become:

<my-tabs id="tabs">
    <my-tab>
        <my-tab-title root-host="tabs" slot="title">
            Title 1
        </my-tab-title>
        <my-tab-content root-host="tabs" slot="content">
            Content 1
        </my-tab-content>
    </my-tab>
    ...
</my-tabs>

Personally, I would like it to be more implicit. I do not think that id should be used for this purpose. In practise, you could not use just tabs because there might be other custom elements of the same type on this page and then the ids would conflict. After copying the markup, you would then always have to adjust it at different places.

@ghost
Copy link
Author

ghost commented Sep 28, 2016

@ebidel in your article about the shadowdom you show how a tabs component can be implemented. Did you choose the "flat markup" approach because of the limitation which is described here or because you think it is good solution?

Copy of his markup:

<fancy-tabs>
  <button slot="title">Title</button>
  <button slot="title" selected>Title 2</button>
  <button slot="title">Title 3</button>
  <section>content panel 1</section>
  <section>content panel 2</section>
  <section>content panel 3</section>
</fancy-tabs>

<!-- Using <h2>'s and changing the ordering would also work! -->
<fancy-tabs>
  <h2 slot="title">Title</h2>
  <section>content panel 1</section>
  <h2 slot="title" selected>Title 2</h2>
  <section>content panel 2</section>
  <h2 slot="title">Title 3</h2>
  <section>content panel 3</section>
</fancy-tabs>

@trusktr
Copy link

trusktr commented Sep 28, 2016

Maybe traversing up is a good compromise in terms of modularity.

On Sep 28, 2016 8:46 AM, "Joel Richard" notifications@github.com wrote:

@trusktr https://github.com/trusktr my tabs example would then become:

Title 1 Content 1 ...

Personally, I would like it to be more implicit. I do not think that id
should be used for this purpose. In practise, you could not use just tabs
because there might be other custom elements of the same type on this page
and then the ids would conflict. After copying the markup, you would then
always have to adjust it at different places.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#574 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AASKzsjTYc215f6gFdZ599L6TGlaNI2fks5quot0gaJpZM4KFPC-
.

@hayatoito
Copy link
Contributor

hayatoito commented Oct 4, 2016

Slotting indirect children does not make sense. The parent of such indirect children is no-op, isn't it? It is never used. You can omit such a parent in any case, and put children as direct children of the shadow host. Nothing changes after that.

My first attempt was to attach a shadow to my-tabs and use a named slot for the titles and contents:

@joelrich
You might want to attach a shadow to <my-tab> too.
You should have two components: <my-tabs> and <my-tab>. Nested components are a common technique, I think.

@ghost
Copy link
Author

ghost commented Oct 4, 2016

@hayatoito, right, the parent would be a no-op. However, this does not make it useless. For example, if you want to remove, move or add a tab, then you could do this with one command. Therefore, this is important to keep the tab titles and contents grouped and synchronized.

Adding a shadow to my-tab does not allow me to achieve the desired composed DOM (see the issue description). If the slot name exists as a slot in the direct parent, then it would behave as normal nested components.

Angular Martial Design uses a similar markup:

Angular 2:

<md-tab-group>
  <md-tab>
    <template md-tab-label>One</template>
    <template md-tab-content>
      <h1>Some tab content</h1>
      <p>...</p>
    </template>
  </md-tab>
  <md-tab>
    <template md-tab-label>Two</template>
    <template md-tab-content>
      <h1>Some more tab content</h1>
      <p>...</p>
    </template>
  </md-tab>
</md-tab-group>

Angular 1:

<md-tabs md-selected="selectedIndex">
  <img ng-src="img/angular.png" class="centered">
  <md-tab ng-repeat="tab in tabs | orderBy:predicate:reversed" md-on-select="onTabSelected(tab)" md-on-deselect="announceDeselected(tab)" ng-disabled="tab.disabled">
    <md-tab-label>
      {{tab.title}}
      <img src="img/removeTab.png" ng-click="removeTab(tab)" class="delete">
    </md-tab-label>
    <md-tab-body>
      {{tab.content}}
    </md-tab-body>
  </md-tab>
</md-tabs>

Please keep in mind that even if <my-tab-title>/<my-tab-content> were just regular elements, it would still not be possible to slot them somewhere else. This is a serious limitation of this specification.

Do you have a better idea how to solve this problem than my suggested algorithm?

@rniwa
Copy link
Collaborator

rniwa commented Oct 5, 2016

I don't think we should close this issue until we've fully explored the problem space.

@rniwa rniwa reopened this Oct 5, 2016
@hayatoito
Copy link
Contributor

hayatoito commented Oct 5, 2016

Adding a shadow to my-tab does not allow me to achieve the desired composed DOM (see the issue description).

I have taken a look the composed DOM in the example, and now I understand why it is impossible. You are correct. Sorry for my misunderstanding.

My opinions for this idea:

  • I am not fan of this idea. If we allow that, markup (in children of the shadow host) does not have meaning. That is not what we want, I think.

For example, suppose that users write the following markup:

<my-tabs>
    <my-tab>
        <my-tab-title>
            Title 1
        </my-tab-title>
        <my-tab-content>
            Content 1
        </my-tab-content>
    </my-tab>
    <my-tba>
        <my-tab-title>
            Title 2
        </my-tab-title>
        <my-tab-content>
            Content 2
        </my-tab-content>
    </my-tab>
</my-tabs>

I guess you did not notice the typo in the markup. They wrote <my-tba>, instead of <my-tab>. However, because this is still correctly rendered, regardless of the mistake, users would not notice their mistakes. That is not what we want to introduce to HTML world, I think. We want appropriate feedback by returning the wrong rendering result if the markup is wrong.

  • This new idea should work with the rest of Shadow DOM APIs. Since a lot of parts depend on the current behavior, it might be hard to make this idea work nicely with the current assumption. For example:
    • How event path should be? The no-op parent should receive an event which happens at its descendant which is assigned to a slot. But it is difficult to have a nice answer how we can calculate an event path in this case.
    • If we allow this, a slotted element would be a descendant of another slotted element. We need yet another sanity check (and new rules) to detect and ban it.
    • Maybe the performance impact might be the most critical issue, though we do not have any data yet. :(

@trusktr
Copy link

trusktr commented Oct 6, 2016

Actually, I don't think parent should be a no-op, and I think the event
path will be almost the same: propagating up the shadow tree (any number of
levels deep), and now propagating up the light tree as well (this part is
new
). The only difference is that the event propagated from the child to
its parent in the light tree. Now, it will simply propagate through all the
ancestors in the light tree. That's not a significant difference.

I have some examples to post in a bit, along with some use cases that are
limited by the current shadow dom spec, but I'm on my phone and need to
finish some manual labor. Will be back soon...
Have not had time.

On Oct 4, 2016 9:32 PM, "Hayato Ito" notifications@github.com wrote:

Adding a shadow to my-tab does not allow me to achieve the desired
composed DOM (see the issue description). If the slot name exists as a slot
in the direct parent, then it would behave as normal nested components.

I have taken a look the composed DOM in the example, and now I understand
why it is impossible. You are correct. Sorry for my misunderstanding.

My opinions for this idea:

  • I am not fan of this idea. If we allow that, markup (in children of
    the shadow host) does not have meaning. That is not what we want, I think.

For example, suppose that users write the following markup:

Title 1 Content 1 Title 2 Content 2

I guess you did not notice the typo in the markup. They wrote ,
instead of . However, because this is still correctly rendered,
regardless of the mistake, users would not notice their mistakes. That is
not what we want to introduce to HTML world, I think. We want correct
feedback by returning the wrong rendering result if the markup is wrong.

  • This new idea should work with the rest of Shadow DOM APIs. For
    example, how event path should be? The no-op parent should receive an event
    which happens at its descendant which is assigned to a slot. But it is
    difficult to have a nice answer how we can calculate an event path in this
    case.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#574 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AASKzm9gtEBN3nuS-A-FtSKusHe-uo1jks5qwydJgaJpZM4KFPC-
.

@hayatoito
Copy link
Contributor

hayatoito commented Oct 6, 2016

@trusktr

I am afraid that I do not understand your idea fully. Could you help me to understand by explaining how get the parent algorithm for nodes and shadow roots should be?

@hayatoito
Copy link
Contributor

hayatoito commented Dec 6, 2016

Let me close this since the idea was not clearly demonstrated.

@dvoytenko
Copy link

We see some use cases related to this with semantical elements, such as <figure>. E.g. if there's a web component that slots a figure and wants to do some special distribution for image vs caption.

E.g.

<web-component-x>
  <figure>
    <img>
    <figcaption>This image describes ...</figcaption>
  </figure>
</web-component-x>

Depending on the composition, it's sometimes too difficult to style the caption with simple CSS changes. It'd be helpful to be able to re-distribute it via a nested slot of some sort. Currently, we often display:none the original figcaption and then clone its content for display purposes within the shadow. This has lots of drawbacks related to the styling and mutations.

@hayatoito
Copy link
Contributor

@dvoytenko Thanks for the feedback.

I am trying to understand the use case. Could you possibly tell us what is the outcome you want more clearly? If we could know the desired composition more clearly, that would be great.

it's sometimes too difficult to style the caption with simple CSS changes. It'd be helpful to be able to re-distribute it via a nested slot of some sort.

It might be a little difficult for me what "nested slot of some sort" means here.

@hayatoito
Copy link
Contributor

It looks this bug was closed. If you need to re-open this bug for the discussion, we are happy to re-open this!

@annevk
Copy link
Collaborator

annevk commented Feb 26, 2019

(I'd vastly prefer a new focused issue without all the 2016 noise.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants