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.

Sign up for GitHub

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 #5483

Closed
wants to merge 15 commits into from
Closed

Add Imperative Slot API #5483

Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
f3f5266
Add Imperative Slot API
Apr 22, 2020
91e0d05
Update Imperative Slot API based on feedback.
Apr 24, 2020
1f10e68
Updates to include manuallyAssignedNodes.
May 14, 2020
1c673df
Update Imperative Slot API html spec.
Jun 5, 2020
a705c37
Update Imperative Slot API spec.
Jun 17, 2020
d30c8f1
Update Imperative Slot API html spec
Oct 16, 2020
854a6b0
Merge remote-tracking branch 'upstream/master'
Oct 16, 2020
b39abb7
Correct syntax errors
Oct 16, 2020
981de6d
Update Imperative Slot API html spec
Nov 12, 2020
ab78ff5
Merge remote-tracking branch 'upstream/master'
Nov 12, 2020
551f474
Merge remote-tracking branch 'upstream/master'
Dec 31, 2020
bbb886a
Update HTML spec for imperative slot API
Dec 31, 2020
174469b
Update Imperative Slot API html spec
Dec 31, 2020
127bb89
Tidy up a bit
domenic Feb 23, 2021
fdc36ca
Handle duplicates and go back to a set
domenic Feb 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 74 additions & 15 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2952,6 +2952,7 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#concept-tree">tree</dfn>, <dfn data-x-href="https://dom.spec.whatwg.org/#concept-shadow-tree">shadow tree</dfn>, and <dfn data-x-href="https://dom.spec.whatwg.org/#concept-node-tree">node tree</dfn> concepts</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#concept-tree-order">tree order</dfn> and <dfn data-x-href="https://dom.spec.whatwg.org/#concept-shadow-including-tree-order">shadow-including tree order</dfn> concepts</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#concept-tree-child" data-x="concept-tree-child">child</dfn> concept</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#concept-tree-descendant" data-x="concept-tree-descendant">descendant</dfn> concept</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#concept-tree-root">root</dfn> and <dfn data-x-href="https://dom.spec.whatwg.org/#concept-shadow-including-root">shadow-including root</dfn> concepts</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#concept-tree-inclusive-ancestor">inclusive ancestor</dfn>,
<dfn data-x-href="https://dom.spec.whatwg.org/#concept-shadow-including-descendant">shadow-including descendant</dfn>,
Expand All @@ -2961,7 +2962,11 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#document-element">document element</dfn> concept</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#in-a-document-tree">in a document tree</dfn>, <dfn data-x-href="https://dom.spec.whatwg.org/#in-a-document">in a document</dfn> (legacy), and <dfn data-x-href="https://dom.spec.whatwg.org/#connected">connected</dfn> concepts</li>
<li>The <dfn data-x="concept-slot" data-x-href="https://dom.spec.whatwg.org/#concept-slot">slot</dfn> concept, and its <dfn data-x="slot-name" data-x-href="https://dom.spec.whatwg.org/#slot-name">name</dfn> and <dfn data-x-href="https://dom.spec.whatwg.org/#slot-assigned-nodes">assigned nodes</dfn></li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#slotable-assigned-slot">assigned slot</dfn> concept.
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#slotable-assigned-slot">assigned slot</dfn> concept</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#dom-shadowroot-slot-assignment">slot assignment</dfn> concept</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#concept-slotable">slottable</dfn> concept</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#assign-slotables-for-a-tree">assign slottables for a tree</dfn> algorithm</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#concept-tree-inclusive-descendant">inclusive descendants</dfn> concept</li>>
<li>The <dfn data-x="finding flattened slottables" data-x-href="https://dom.spec.whatwg.org/#find-flattened-slotables">find flattened slottables</dfn> algorithm</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#assign-a-slot">assign a slot</dfn> algorithm</li>
<li>The <dfn data-x-href="https://dom.spec.whatwg.org/#concept-node-pre-insert">pre-insert</dfn>, <dfn data-x="concept-node-insert" data-x-href="https://dom.spec.whatwg.org/#concept-node-insert">insert</dfn>, <dfn data-x="concept-node-append" data-x-href="https://dom.spec.whatwg.org/#concept-node-append">append</dfn>, <dfn data-x="concept-node-replace" data-x-href="https://dom.spec.whatwg.org/#concept-node-replace">replace</dfn>, <dfn data-x="concept-node-replace-all" data-x-href="https://dom.spec.whatwg.org/#concept-node-replace-all">replace all</dfn>, <dfn data-x-href="https://dom.spec.whatwg.org/#string-replace-all">string replace all</dfn>, <dfn data-x="concept-node-remove" data-x-href="https://dom.spec.whatwg.org/#concept-node-remove">remove</dfn>, and <dfn data-x="concept-node-adopt" data-x-href="https://dom.spec.whatwg.org/#concept-node-adopt">adopt</dfn> algorithms for nodes</li>
Expand Down Expand Up @@ -59244,16 +59249,17 @@ interface <dfn>HTMLTemplateElement</dfn> : <span>HTMLElement</span> {
<dt><span data-x="concept-element-dom">DOM interface</span>:</dt>
<dd w-nodev>
<pre><code class="idl">[Exposed=Window]
interface <dfn>HTMLSlotElement</dfn> : <span>HTMLElement</span> {
interface <dfn interface>HTMLSlotElement</dfn> : <span>HTMLElement</span> {
[<span>HTMLConstructor</span>] constructor();

[<span>CEReactions</span>] attribute DOMString <span data-x="dom-slot-name">name</span>;
sequence&lt;Node> <span data-x="dom-slot-assignedNodes">assignedNodes</span>(optional <span>AssignedNodesOptions</span> options = {});
sequence&lt;Element> <span data-x="dom-slot-assignedElements">assignedElements</span>(optional <span>AssignedNodesOptions</span> options = {});
undefined <span data-x="dom-slot-assign">assign</span>((<span>Element</span> or <span>Text</span>)... nodes);
};

dictionary <dfn>AssignedNodesOptions</dfn> {
boolean flatten = false;
dictionary <dfn dictionary>AssignedNodesOptions</dfn> {
boolean <dfn dict-member for="AssignedNodeOptions" data-x="dom-AssignedNodesOptions-flatten">flatten</dfn> = false;
};</code></pre>
</dd>
<dd w-dev>Uses <code>HTMLSlotElement</code>.</dd>
Expand Down Expand Up @@ -59295,33 +59301,85 @@ dictionary <dfn>AssignedNodesOptions</dfn> {
<dt><var>slot</var> . <code data-x="dom-slot-assignedElements">assignedElements</code>({ flatten: true })</dt>
<dd>Returns the same as <code data-x="dom-slot-assignedNodes">assignedNodes({ flatten: true
})</code>, limited to elements.</dd>

<dt><var>slot</var> . <code data-x="dom-slot-assign">assign</code>(...nodes)</dt>
<dd>
<p>Set <var>slot</var>'s <span>assigned nodes</span> to the given <var>nodes</var>.</p>

<p>Throws a <span>"<code>NotAllowedError</code>"</span> <code>DOMException</code> if
<var>slot</var>'s <span>root</span> is not a <span>shadow root</span>, its <span>root</span>'s
<span>slot assignment</span> is not "manual", or <var>nodes</var> are not <span>slottable</span>
children of <var>slot</var>'s <span>root</span>'s <span>shadow host</span>.</p>
</dd>
</dl>

<p>The <dfn data-x="dom-slot-name"><code>name</code></dfn> IDL attribute must <span>reflect</span>
the content attribute of the same name.</p>

<p>The <dfn data-x="dom-slot-assignedNodes"><code>assignedNodes(<var>options</var>)</code></dfn>
method, when invoked, must run these steps:</p>
method steps are:</p>

<ol>
<li><p>If the value of <var>options</var>'s <code data-x="">flatten</code> member is false, then
return this element's <span>assigned nodes</span>.</p></li>
<li><p>If <var>options</var>["<code data-x="dom-AssignedNodesOptions-flatten">flatten</code>"] is
false, then return <span>this</span>'s <span>assigned nodes</span>.</p></li>

<li><p>Return the result of <span>finding flattened slottables</span> with this element.</p></li>
<li><p>Return the result of <span>finding flattened slottables</span> with
<span>this</span>.</p></li>
</ol>

<p>The <dfn data-x="dom-slot-assignedElements"><code>assignedElements(<var>options</var>)</code></dfn>
method, when invoked, must run these steps:</p>
<p>The <dfn
data-x="dom-slot-assignedElements"><code>assignedElements(<var>options</var>)</code></dfn> method
steps are:</p>

<ol>
<li><p>If the value of <var>options</var>'s <code data-x="">flatten</code> member is false, then
return this element's <span>assigned nodes</span>, filtered to contain only <code>Element</code>
nodes.</p></li>
<li><p>If <var>options</var>["<code data-x="dom-AssignedNodesOptions-flatten">flatten</code>"] is
false, then return <span>this</span>'s <span>assigned nodes</span>, filtered to contain only
<code>Element</code> nodes.</p></li>

<li><p>Return the result of <span>finding flattened slottables</span> with this element, filtered
to contain only <code>Element</code> nodes.</p></li>
<li><p>Return the result of <span>finding flattened slottables</span> with <span>this</span>,
filtered to contain only <code>Element</code> nodes.</p></li>
</ol>

<p>The <code>slot</code> element has <dfn export for="HTMLSlotElement">manually assigned
nodes</dfn>, which is an <span data-x="set">ordered set</span> of <span
data-x="slottable">slottables</span> set by <code data-x="dom-slot-assign">assign()</code>. It is
initially empty.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Should we move this up? Usually we describe "slots" before public members.

Copy link
Contributor

Choose a reason for hiding this comment

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

I moved this up above the assignedNodes method steps. Let me know if that's what you were requesting here.


<p>The <dfn method for="HTMLSlotElement"
data-x="dom-slot-assign"><code>assign(...<var>nodes</var>)</code></dfn> method steps are:</p>

<ol>
domenic marked this conversation as resolved.
Show resolved Hide resolved
<li><p>If <span>this</span>'s <span>root</span> is not a <span>shadow root</span>, or its
<span>slot assignment</span> is not "<code data-x="">manual</code>", then throw a
<span>"<code>NotAllowedError</code>"</span> <code>DOMException</code>.</p></li>

<li><p><span data-x="list iterate">For each</span> <var>node</var> of <var>nodes</var>: if
<var>node</var> is not a <span data-x="concept-tree-child">child</span> of <span>this</span>'s
<span>root</span>'s <span>shadow host</span>, then throw a
<span>"<code>NotAllowedError</code>"</span> <code>DOMException</code>.</p></li>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm going to leave this whole thing here - it seems reasonable to me to throw an exception if, at assign() call time, the provided children aren't in the right place. If they're later removed from the shadow host, we will just pretend they're not there. If they're ever put back, we'll start using them again. But I don't know what the use case would be for assigning nodes that aren't present yet (perhaps not even in the same document). Does that sound ok?

For this reason, I don't think there are any changes needed to this PR to incorporate the proposed changes.

Copy link

Choose a reason for hiding this comment

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

Hm... what if you wanted to assign nodes then add them as children of the shadow host? The one of main use cases of manual assignment is so that custom element can have JS API which accepts a list of nodes to use, right? In that scenario, it's possible for the users of a custom element to create a bunch of nodes, call JS API, then add them as children of the host instead of adding them first and then giving them to the JS API. At least, that was one of the use cases that came up during accessibility related discussion in the past which motivated the current design.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's a good point. Ok, sounds like we should probably remove the second <li> here that throws an exception if node isn't a child of the shadow host. I'll leave the first check that the <slot> is within a shadow root and that it is in manual mode.


<li><p>Let <var>nodesSet</var> be a new <span data-x="set">ordered set</span>.</p></li>

<li>
<p><span data-x="list iterate">For each</span> <var>node</var> of <var>nodes</var>:</p>

<ol>
<li><p><span data-x="list iterate">For each</span> <span
data-x="concept-tree-descendant">descendant</span> <var>descendant</var> of <span>this</span>'s
<span>root</span>, in <span>tree order</span>: if <var>descendant</var> is a <span>slot</span>,
then <span data-x="list remove">remove</span> <var>node</var> from <var>descendant</var>'s
<span>manually assigned nodes</span>.</p></li>

Copy link

Choose a reason for hiding this comment

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

This isn't quite right. We need to remove duplicates from the manually assigned nodes since "find slottables" in https://github.com/whatwg/dom/pull/860/files doesn't remove duplicates either.

Copy link
Member

@domenic domenic Feb 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, that seems more correct to me, and seems to match the implementation in Chromium. https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/core/html/html_slot_element.cc;l=187;drc=510d6b8791a0e20c3470f9c0fcf1472d1bb71995

I will update the PR.

Copy link
Member

Choose a reason for hiding this comment

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

It seems slightly cleaner to make the set during the first iteration to avoid redundant descendant searches, but I think it should not be observable...

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree here that we should unique the list here. The Chromium implementation also has some logic around ordering in the case of duplicates, which I think makes sense. I'll try to write that up here. domenic@, you mentioned you'd update the PR, that didn't happen, right? Just making sure I don't step on a change for that.

Copy link
Member

Choose a reason for hiding this comment

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

I did update the PR; see commit fdc36ca

<li><p><span data-x="set append">Append</span> <var>node</var> to <var>nodesSet</var>.</p></li>
</ol>
</li>

<li><p>Set <span>this</span>'s <span>manually assigned nodes</span> to
<var>nodesSet</var>.</p></li>

<li><p>Run <span>assign slottables for a tree</span> for <span>this</span>'s
<span>root</span>.</p></li>
</ol>


<h4 split-filename="canvas">The <dfn id="canvas"><code>canvas</code></dfn> element</h4>
Expand Down Expand Up @@ -123980,6 +124038,7 @@ INSERT INTERFACES HERE
Yngve Nysaeter Pettersen,
Yoav Weiss,
Yonathan Randolph,
Yu Han, <!-- yuzhe-han on GitHub -->
Yu Huojiang,
Yury Delendik,
平野裕 (Yutaka Hirano),
Expand Down