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

Add Imperative Slot API #860

Closed
wants to merge 17 commits into from
39 changes: 35 additions & 4 deletions dom.bs
Original file line number Diff line number Diff line change
Expand Up @@ -2185,6 +2185,10 @@ steps:</p>
<li><p>If the <i>open flag</i> is set and <var>shadow</var>'s <a for=ShadowRoot>mode</a> is
<em>not</em> "<code>open</code>", then return null.</p></li>

<li><p>If <var>shadow</var>'s <a for=ShadowRoot>slot assignment</a> is "<code>manual</code>",
then return the <a>slot</a> in <var>shadow</var>'s <a for=tree>descendants</a> whose <a>manually assigned nodes</a>
<a for=set>contains</a> <var>slottable</var>, if any, and null otherwise.</p></li>
domenic marked this conversation as resolved.
Show resolved Hide resolved

<li><p>Return the first <a>slot</a> in <a>tree order</a> in <var>shadow</var>'s
<a for=tree>descendants</a> whose <a for=slot>name</a> is <var>slottable</var>'s
<a for=slottable>name</a>, if any, and null otherwise.</p></li>
Expand Down Expand Up @@ -2440,7 +2444,8 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
<li><p>Otherwise, <a for=set>insert</a> <var>node</var> into <var>parent</var>'s
<a for=tree>children</a> before <var>child</var>'s <a for=tree>index</a>.

<li><p>If <var>parent</var> is a <a for=Element>shadow host</a> and <var>node</var> is a
<li><p>If <var>parent</var> is a <a for=Element>shadow host</a> whose <a for=/>shadow root</a>'s
<a for=ShadowRoot>slot assignment</a> is "<code>auto</code>" and <var>node</var> is a
<a>slottable</a>, then <a>assign a slot</a> for <var>node</var>.

<li><p>If <var>parent</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a>, and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should this step not be skipped?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step,
** If parent’s root is a shadow root, and parent is a slot whose assigned nodes is the empty list, then run signal a slot change for parent. **
is to signal slot change event because the slot's fallback content has changed.

For slot assignment "manual", the slot still supports fallback content.

Expand Down Expand Up @@ -2657,8 +2662,16 @@ indicated in the <a for=/>remove</a> algorithm below.

<li><p><a for=set>Remove</a> <var>node</var> from its <var>parent</var>'s <a for=tree>children</a>.

<li><p>If <var>node</var> is <a for=slottable>assigned</a>, then run <a>assign slottables</a> for
<var>node</var>'s <a>assigned slot</a>.
<li>
<p>If <var>node</var> is <a for=slottable>assigned</a>, then:

<ol>
<li><p>If <var>parent</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a> whose
<a for=tree>root</a>'s <a for=ShadowRoot>slot assignment</a> is "<code>manual</code>",
then <a for=set>remove</a> <var>node</var> from its <a>assigned slot</a>'s <a>manually assigned nodes</a>.

<li><p>Run <a>assign slottables</a> for <var>node</var>'s <a>assigned slot</a>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too I have the feeling we're not skipping enough for the manual case. Same below.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how much optimization can be done here. Once the assigned node has been removed from slot's manually assigned node, we need to recalculate the slot's assigned nodes and signal slot change event. Both occurs inside assign slottables.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that's fair.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can this be right? assign slottables would set the list of assigned nodes to slottable matching the slot in tree order. It would mean that we're always re-order slottable in the tree order regardless of in what order they're inserted.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that. I've updated the find-slottables algorithm to preserve the order of manually assigned nodes.

</ol>

<li><p>If <var>parent</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a>, and
<var>parent</var> is a <a>slot</a> whose <a for=slot>assigned nodes</a> is the empty list,
Expand All @@ -2668,6 +2681,11 @@ indicated in the <a for=/>remove</a> algorithm below.
<p>If <var>node</var> has an <a>inclusive descendant</a> that is a <a>slot</a>, then:

<ol>
<li><p>for each <a>slot</a> <var>slot</var> in <var>node</var>'s <a for=tree>inclusive descendants</a>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<li><p>for each <a>slot</a> <var>slot</var> in <var>node</var>'s <a for=tree>inclusive descendants</a>,
<li><p>For each <a>slot</a> <var>slot</var> in <var>node</var>'s <a for=tree>inclusive descendants</a>,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if <var>slot</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a> whose <a for=tree>root</a>'s
<a for=ShadowRoot>slot assignment</a> is "<code>manual</code>", then set <var>slot</var>'s
<a>manually assigned nodes</a> to an empty set.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... this would be different from what we've been discussing in AOM:
whatwg/html#4925

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this relate to that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it would mean that that this slot.assingedNodes would behave differently from element ID reflection, which seems bad.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.


<li><p>Run <a>assign slottables for a tree</a> with <var>parent</var>'s <a for=tree>root</a>.

<li><p>Run <a>assign slottables for a tree</a> with <var>node</var>.
Expand Down Expand Up @@ -5701,11 +5719,13 @@ invoked, must return a new {{DocumentFragment}} <a>node</a> whose <a for=Node>no
[Exposed=Window]
interface ShadowRoot : DocumentFragment {
readonly attribute ShadowRootMode mode;
readonly attribute SlotAssignmentMode slotAssignment;
readonly attribute Element host;
attribute EventHandler onslotchange;
};

enum ShadowRootMode { "open", "closed" };
enum SlotAssignmentMode { "manual", "auto" };
annevk marked this conversation as resolved.
Show resolved Hide resolved
</pre>

<p>{{ShadowRoot}} <a for=/>nodes</a> are simply known as
Expand All @@ -5724,6 +5744,9 @@ It is initially set to false.</p>
<!-- If we ever change this, e.g., add a ShadowRoot object constructor, that would have serious
consequences for innerHTML. -->

<p><a for=/>Shadow roots</a> have an associated <dfn for=ShadowRoot>slot assignment</dfn>
("<code>manual</code>" or "<code>auto</code>").</p>

<p>A <a for=/>shadow root</a>'s <a>get the parent</a> algorithm, given an <var>event</var>, returns
null if <var>event</var>'s <a>composed flag</a> is unset and <a for=/>shadow root</a> is the
<a for=tree>root</a> of <var>event</var>'s <a for=Event>path</a>'s first struct's
Expand All @@ -5741,6 +5764,9 @@ null if <var>event</var>'s <a>composed flag</a> is unset and <a for=/>shadow roo
<dfn for=ShadowRoot export><code>onslotchange</code></dfn> <a>event handler</a>, whose
<a>event handler event type</a> is {{HTMLSlotElement/slotchange}}.

<p>The <dfn attribute for=ShadowRoot><code>slotAssignment</code></dfn> attribute's getter must
return <a>this</a>'s <a for=ShadowRoot>slot assignment</a>.</p>

<hr>

<p>In <dfn export id=concept-shadow-including-tree-order>shadow-including tree order</dfn> is
Expand Down Expand Up @@ -5864,6 +5890,7 @@ interface Element : Node {
dictionary ShadowRootInit {
required ShadowRootMode mode;
boolean delegatesFocus = false;
SlotAssignmentMode slotAssignment = "auto";
};
</pre>

Expand Down Expand Up @@ -6748,6 +6775,9 @@ invoked, must run these steps:
"<code>custom</code>", then set <var>shadow</var>'s
<a for=ShadowRoot>available to element internals</a> to true.

<li><p>Set <var>shadow</var>'s <a for=ShadowRoot>slot assignment</a> to <var>init</var>'s
{{ShadowRootInit/slotAssignment}}.

<li><p>Set <a>this</a>'s <a for=Element>shadow root</a> to <var>shadow</var>.

<li><p>Return <var>shadow</var>.
Expand Down Expand Up @@ -10193,7 +10223,8 @@ Yash Handa,
Yehuda Katz,
Yoav Weiss,
Yoichi Osato,
Yoshinori Sano, and
Yoshinori Sano,
Yu Han, and
Zack Weinberg
for being awesome!

Expand Down