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 name attribute for grouping details elements into an exclusive accordion #9400

Merged
merged 14 commits into from
Oct 2, 2023
Merged
Changes from all commits
Commits
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
277 changes: 261 additions & 16 deletions source
Original file line number Diff line number Diff line change
Expand Up @@ -10697,6 +10697,14 @@ partial interface <dfn id="document" data-lt="">Document</dfn> {
<p class="note">This is only populated for "<code data-x="">about:</code>"-schemed
<code>Document</code>s.</p>

<p>Each <code>Document</code> has a <dfn data-x="concept-document-fire-mutation-events-flag">fire
mutation events flag</dfn>, which is a boolean, initially true.</p>

<p class="note">This is intended to suppress firing of DOM Mutation Events in cases when they
would normally fire. The specification describing mutation events is not actively maintained so
it does not look at this flag, but implementations are expected to act as though it did.
<ref>UIEVENTS</ref></p>
dbaron marked this conversation as resolved.
Show resolved Hide resolved

<h4>The <code>DocumentOrShadowRoot</code> interface</h4>

<p><cite>DOM</cite> defines the <code data-x="DOM
Expand Down Expand Up @@ -59906,6 +59914,7 @@ dictionary <dfn dictionary>FormDataEventInit</dfn> : <span>EventInit</span> {
<dd>One <code>summary</code> element followed by <span>flow content</span>.</dd>
<dt><span data-x="concept-element-attributes">Content attributes</span>:</dt>
<dd><span>Global attributes</span></dd>
<dd><code data-x="attr-details-name">name</code></dd>
<dd><code data-x="attr-details-open">open</code></dd>
<dt><span
data-x="concept-element-accessibility-considerations">Accessibility considerations</span>:</dt>
Expand All @@ -59917,6 +59926,7 @@ dictionary <dfn dictionary>FormDataEventInit</dfn> : <span>EventInit</span> {
interface <dfn interface>HTMLDetailsElement</dfn> : <span>HTMLElement</span> {
[<span>HTMLConstructor</span>] constructor();

[<span>CEReactions</span>] attribute DOMString <span data-x="dom-details-name">name</span>;
[<span>CEReactions</span>] attribute boolean <span data-x="dom-details-open">open</span>;
};</code></pre>
</dd>
Expand All @@ -59937,6 +59947,31 @@ interface <dfn interface>HTMLDetailsElement</dfn> : <span>HTMLElement</span> {
<p>The rest of the element's contents <span>represents</span> the additional information or
controls.</p>

<p>The <dfn element-attr for="details"><code data-x="attr-details-name">name</code></dfn> content
attribute gives the name of the group of related <code>details</code> elements that the element is
a member of. Opening one member of this group causes other members of the group to close. If the
attribute is specified, its value must not be the empty string.</p>

<p>A document must not contain more than one <code>details</code> element in the same
<span>details name group</span> that has the <code data-x="attr-details-open">open</code>
attribute present. Authors must not use script to add <code>details</code> elements to a document
in a way that would cause a <span>details name group</span> to have more than one
<code>details</code> element with the <code data-x="attr-details-open">open</code> attribute
present.</p>

<p class="note">The group of elements that is created by a common <code
data-x="attr-details-name">name</code> attribute is exclusive, meaning that at most one of the
<code>details</code> elements can be open at once. While this exclusivity is enforced by user
agents, the resulting enforcement immediately changes the <code
data-x="attr-details-open">open</code> attributes in the markup. This requirement on authors
forbids such misleading markup.</p>

<p>Documents that use the <code data-x="attr-details-name">name</code> attribute to group multiple
related <code>details</code> elements should keep those related elements together in a containing
element (such as a <code>section</code> element).</p>

<p class="note">Keeping related elements together can be important for accessibility.</p>

<p>The <dfn element-attr for="details"><code data-x="attr-details-open">open</code></dfn> content
attribute is a <span>boolean attribute</span>. If present, it indicates that both the summary and
the additional information is to be shown to the user. If the attribute is absent, only the
Expand All @@ -59963,25 +59998,68 @@ interface <dfn interface>HTMLDetailsElement</dfn> : <span>HTMLElement</span> {
exists, user agents can still provide this ability through some other user interface
affordance.</p>

<p>The <dfn>details name group</dfn> that contains a <code>details</code> element <var>a</var>
also contains all the other <code>details</code> elements <var>b</var> that fulfill all of the
following conditions:</p>

<ul>
<li>Both <var>a</var> and <var>b</var> are in the same <span>tree</span>.</li>

<li>They both have a <code data-x="attr-details-name">name</code> attribute, their <code
data-x="attr-details-name">name</code> attributes are not the empty string, and the value of
<var>a</var>'s <code data-x="attr-details-name">name</code> attribute equals the value of
<var>b</var>'s <code data-x="attr-details-name">name</code> attribute.</li>
</ul>

<p>Every <code>details</code> element has a <dfn>details toggle task tracker</dfn>, which is a
<span>toggle task tracker</span> or null, initially null.</p>

<p>Whenever the <code data-x="attr-details-open">open</code> attribute is added to or removed from
a <code>details</code> element, the user agent must run the following steps, which are known as
the <dfn>details notification task steps</dfn>, for this <code>details</code> element:</p>

<p class="note">When the <code data-x="attr-details-open">open</code> attribute is toggled several
times in succession, the resulting tasks essentially get coalesced so that only one event is
fired.</p>
<p>The following <span data-x="concept-element-attributes-change-ext">attribute change
steps</span>, given <var>element</var>, <var>localName</var>, <var>oldValue</var>,
<var>value</var>, and <var>namespace</var>, are used for all <code>details</code> elements:</p>

dbaron marked this conversation as resolved.
Show resolved Hide resolved
<ol>
<li><p>If the <code data-x="attr-details-open">open</code> attribute is added, <span>queue a
details toggle event task</span> given the <code>details</code> element, "<code
data-x="">closed</code>", and "<code data-x="">open</code>".</p></li>
<li><p>If <var>namespace</var> is not null, then return.</p></li>

<li><p>If <var>localName</var> is <code data-x="attr-details-name">name</code>, then <span>ensure
details exclusivity by closing the given element if needed</span> given
<var>element</var>.</p></li>

<li><p>Otherwise, <span>queue a details toggle event task</span> given the <code>details</code>
element, "<code data-x="">open</code>", and "<code data-x="">closed</code>".</p></li>
<li><p>If <var>localName</var> is <code data-x="attr-details-open">open</code>, then:
<ol>
<li>
<p>If one of <var>oldValue</var> or <var>value</var> is null and the other is not null,
run the following steps, which are known as the <dfn>details notification task steps</dfn>, for
this <code>details</code> element:</p>

<p class="note">When the <code data-x="attr-details-open">open</code> attribute is toggled
several times in succession, the resulting tasks essentially get coalesced so that only one
event is fired.</p>

<ol>
<li><p>If <var>oldValue</var> is null, <span>queue a details toggle event task</span> given
the <code>details</code> element, "<code data-x="">closed</code>", and "<code
data-x="">open</code>".</p></li>

<li><p>Otherwise, <span>queue a details toggle event task</span> given the
<code>details</code> element, "<code data-x="">open</code>", and "<code
data-x="">closed</code>".</p></li>
</ol>
</li>

<li><p>If <var>oldValue</var> is null and <var>value</var> is not null, then <span>ensure
details exclusivity by closing other elements if needed</span> given
<var>element</var>.</p></li>
</ol>
</li>
</ol>

<p>The <code>details</code> <span data-x="html element insertion steps">HTML element insertion
steps</span>, given <var>insertedNode</var>, are:</p>
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, should we just suppress mutation events for these changes too??

Copy link
Member Author

@dbaron dbaron Sep 28, 2023

Choose a reason for hiding this comment

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

I don't feel like that's necessary. I think copying a list before sending the items notifications is a reasonably common pattern (although maybe it became common because of things like mutation events).

Also, if the exclusivity enforcement is actually 100% foolproof, this isn't really even needed anymore because we could change the spec to return from the algorithm when finding the existing open details element. I'm not sure I'm ready to presume that it's 100% foolproof, although I admit I don't know how to break it.

So if I were to change something here I'd probably be inclined towards changing it to assume that the exclusivity enforcement works and just toggle the first open details that it finds and then return.

[Edit: but I'm inclined to leave it as-is.]

Copy link
Member

@domenic domenic Sep 29, 2023

Choose a reason for hiding this comment

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

I meant the suggestion more from the perspective, discussed a bit at TPAC, that mutation events are bad and we should maybe take the opportunity to turn them off with new features in general. In particular, it might be more intuitive and easy to explain that "mutation events don't work with named <details>", instead of "mutation events don't work when inserting details might change the name, but do work for other name-related mutations".

I don't feel strongly on this and am happy to leave it up to you, although @mfreed7 might also be interested.

Concretely, my suggestion is to include the flipping inside both the "ensure details exclusivity by closing other elements if needed" and "ensure details exclusivity by closing the given element if needed" algorithms, so that whenever they are called, no mutation events result. Right now it's just flipped for a single call site of the latter.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've done that -- it seems reasonable to me. It does make sense from the perspective of explaining this to authors (though it does also apply the change to cases that they're more likely to hit).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for suggesting this @domenic. I agree for two reasons: first, I like the TPAC suggestion to make cool new features disable mutation events more, as a carrot. And second, as you both mentioned, it feels easier to just say "mutation events don't fire for named details" than the more complicated thing. So +1.

I'm almost inclined to re-suggest Domenic's point at TPAC - perhaps adding a <details name> in the page disables all mutation events for that document from that point on. That's pretty heavy-handed, so I'd understand if you don't want to do that. But I'd be supportive if you did! 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is pretty heavy-handed -- that sort of interaction can make things difficult for pages that are built in modular ways and combined. I think it's dangerous for global behavior changes to come from something that looks entirely local -- if we're going to have global behavior toggles they should look global.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is pretty heavy-handed -- that sort of interaction can make things difficult for pages that are built in modular ways and combined. I think it's dangerous for global behavior changes to come from something that looks entirely local -- if we're going to have global behavior toggles they should look global.

I assume you're talking more about my second paragraph ("from that point on") and not the first ("just for details mutations"), right? If so, I see your point and agree. My second paragraph was just an underhanded way to spread the pain of the mutation events deprecation. 😵‍💫 Nicely played avoiding that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I was replying to the second paragraph!


<ol>
<li><p><span>Ensure details exclusivity by closing the given element if needed</span> given
<var>insertedNode</var>.</p></li>
</ol>

<p>To <dfn>queue a details toggle event task</dfn> given a <code>details</code> element
Expand Down Expand Up @@ -60023,9 +60101,105 @@ interface <dfn interface>HTMLDetailsElement</dfn> : <span>HTMLElement</span> {
to <var>oldState</var>.</p></li>
</ol>

<p>The <dfn attribute for="HTMLDetailsElement"><code data-x="dom-details-open">open</code></dfn>
IDL attribute must <span>reflect</span> the <code data-x="attr-details-open">open</code> content
attribute.</p>
<p>To <dfn>ensure details exclusivity by closing other elements if needed</dfn> given a
<code>details</code> element <var>element</var>:</p>

<ol>
<li><p><span>Assert</span>: <var>element</var> has an <code
data-x="attr-details-open">open</code> attribute.</p></li>

<!-- This step is an optimization, but it may also make things clearer. -->
<li><p>If <var>element</var> does not have a <code data-x="attr-details-name">name</code>
attribute, or its <code data-x="attr-details-name">name</code> attribute is the empty string,
then return.</p></li>

<li><p>Let <var>document</var> be <var>element</var>'s <span>node document</span>.</p></li>

<li><p>Let <var>oldFlag</var> be the value of <var>document</var>'s <span
data-x="concept-document-fire-mutation-events-flag">fire mutation events flag</span>.</p></li>

<li><p>Set <var>document</var>'s <span data-x="concept-document-fire-mutation-events-flag">fire
mutation events flag</span> to false.</p></li>

<li><p>Let <var>groupMembers</var> be a list of elements, containing all elements in
<var>element</var>'s <span>details name group</span> except for <var>element</var>, in <span>tree
order</span>.</p></li>

<li>
<p><span data-x="list iterate">For each</span> element <var>otherElement</var> of
<var>groupMembers</var>:</p>
<ol>
<li>
<p>If the <code data-x="attr-details-open">open</code> attribute is set on
<var>otherElement</var>, then:</p>

<ol>
<li><p><span>Assert</span>: <var>otherElement</var> is the only element in
dbaron marked this conversation as resolved.
Show resolved Hide resolved
<var>groupMembers</var> that has the <code data-x="attr-details-open">open</code> attribute
set.</p></li>

<li><p><span data-x="concept-element-attributes-remove">Remove</span> the <code
data-x="attr-details-open">open</code> attribute on <var>otherElement</var>.</p></li>

<li><p><span>Break</span>.</p></li>
</ol>
</ol>
</li>

<li><p>Set <var>document</var>'s <span data-x="concept-document-fire-mutation-events-flag">fire
mutation events flag</span> to <var>oldFlag</var>.</p></li>
</ol>

<p>To <dfn>ensure details exclusivity by closing the given element if needed</dfn> given a
<code>details</code> element <var>element</var>:</p>
dbaron marked this conversation as resolved.
Show resolved Hide resolved

<ol>
<li><p>If <var>element</var> does not have an <code data-x="attr-details-open">open</code>
attribute, then return.</p></li>

<!-- This step is an optimization, but it may also make things clearer. -->
<li><p>If <var>element</var> does not have a <code data-x="attr-details-name">name</code>
attribute, or its <code data-x="attr-details-name">name</code> attribute is the empty string,
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to make it clear how the behavior is different for the "ensure details exclusivity after mutation" vs. the attribute change steps. In particular, based on the name of "ensure details exclusivity after mutation", I would have assumed you could call that from the attribute change steps.

One suggestion:

  • Introduce two algorithms, "ensure details exclusivity by removing the given element's open if needed" and "ensure details exclusivity by removing other elements' open if needed". The latter will only have one call site, but I think having the two exist side-by-side is more helpful.
  • Add some <div class="example">s showing the before/after of the three different scenarios: changing name="", changing open="", and inserting a <details>.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've done this roughly as you described, with the following exceptions:

  • I slightly shortened your proposed names to "ensure details exclusivity by closing the given element if needed" and "ensure details exclusivity by closing other elements if needed", though they're still long!
  • I only added an example for the case of changing an open since I think examples should generally demonstrate good practice, and I think the handling for the other cases is really about maintaining the exclusivity invariant in the face of bad practice. (This is much of what [exclusive accordion] exclusively non-exclusive... openui/open-ui#786 was about.)

Copy link
Member

Choose a reason for hiding this comment

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

This looks great! Can we unify the algorithm styles a little bit? Steps 2, 3, and 3.1 of "ensure details exclusivity by closing other elements if needed" are almost identical to steps 2, 3, 3.1, and 3.1.1 of "ensure details exclusivity by closing the given element if needed", but use rather different styles and wording.

I'd suggest something like:

  1. Let groupMembers be a list of elements, containing all elements in element's details name group except for element, in tree order.
  2. For each element otherElement of groupMembers:
    1. If the open attribute is set on otherElement, then:
      1. Assert: otherElement is the only element in groupMembers that has the open attribute set.
      2. Remove the open attribute on ...varies per algorithm...
      3. Return.

As for the examples, I thought including the bad-case examples would be helpful for implementers to understand. I don't insist though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've done the unification you described. (It also had some interactions with the mutation events suppression.)

I think my inclination on examples is that, for implementers, having the examples in WPT is more useful than having them in the spec. :-) So I'd still lean against having them in the spec. And I think the necessary test for name attribute changes is covered in https://wpt.fyi/results/html/semantics/interactive-elements/the-details-element/name-attribute.tentative.html . I realize I need to add a test there for the insertion-while-open case.

then return.</p></li>

<li><p>Let <var>document</var> be <var>element</var>'s <span>node document</span>.</p></li>

<li><p>Let <var>oldFlag</var> be the value of <var>document</var>'s <span
data-x="concept-document-fire-mutation-events-flag">fire mutation events flag</span>.</p></li>

<li><p>Set <var>document</var>'s <span data-x="concept-document-fire-mutation-events-flag">fire
mutation events flag</span> to false.</p></li>

<li><p>Let <var>groupMembers</var> be a list of elements, containing all elements in
<var>element</var>'s <span>details name group</span> except for <var>element</var>, in <span>tree
order</span>.</p></li>

<li>
<p><span data-x="list iterate">For each</span> element <var>otherElement</var> of
<var>groupMembers</var>:</p>

<ol>
<li>
<p>If the <code data-x="attr-details-open">open</code> attribute is set on
<var>otherElement</var>, then:</p>

<ol>
<li><p><span data-x="concept-element-attributes-remove">Remove</span> the <code
data-x="attr-details-open">open</code> attribute on <var>element</var>.</p></li>

<li><p><span>Break</span>.</p></li>
</ol>
</li>
</ol>
</li>

<li><p>Set <var>document</var>'s <span data-x="concept-document-fire-mutation-events-flag">fire
mutation events flag</span> to <var>oldFlag</var>.</p></li>
</ol>

<p>The <dfn attribute for="HTMLDetailsElement"><code data-x="dom-details-name">name</code></dfn>
and <dfn attribute for="HTMLDetailsElement"><code data-x="dom-details-open">open</code></dfn>
IDL attributes must <span>reflect</span> the respective content attributes of the same name.</p>

</div>

Expand Down Expand Up @@ -60102,6 +60276,62 @@ interface <dfn interface>HTMLDetailsElement</dfn> : <span>HTMLElement</span> {

</div>

<div class="example" id="example-details-exclusive-accordion">
dbaron marked this conversation as resolved.
Show resolved Hide resolved
<p>The following example shows the <code data-x="attr-details-name">name</code> attribute of the
<code>details</code> element being used to create an exclusive accordion, a set of
<code>details</code> elements where a user action to open one <code>details</code> element causes
any open <code>details</code> to close.</p>

<pre><code class="html">&lt;section class="characteristics">
&lt;details name="frame-characteristics">
&lt;summary>Material&lt;/summary>
The picture frame is made of solid oak wood.
&lt;/details>
&lt;details name="frame-characteristics">
&lt;summary>Size&lt;/summary>
The picture frame fits a photo 40cm tall and 30cm wide.
The frame is 45cm tall, 35cm wide, and 2cm thick.
&lt;/details>
&lt;details name="frame-characteristics">
&lt;summary>Color&lt;/summary>
The picture frame is available in its natural wood
color, or with black stain.
&lt;/details>
&lt;/section></code></pre>
</div>

<div class="example" id="example-details-exclusive-accordion-setting-open">
<p>The following example shows what happens when the <code data-x="attr-details-open">open</code>
attribute is set on a <code>details</code> element that is part of a set of elements using the
<code data-x="attr-details-name">name</code> attribute to create an exclusive accordion.</p>

<p>Given the initial markup:</p>

<pre><code class="html">&lt;section class="characteristics">
&lt;details name="frame-characteristics" id="d1" open>...&lt;/details>
&lt;details name="frame-characteristics" id="d2">...&lt;/details>
&lt;details name="frame-characteristics" id="d3">...&lt;/details>
&lt;/section></code></pre>

<p>and the script:</p>

<pre><code class="js">document.getElementById("d2").setAttribute("open", "");</code></pre>

<p>then the resulting tree after the script executes will be equivalent to the markup:</p>

<pre><code class="html">&lt;section class="characteristics">
&lt;details name="frame-characteristics" id="d1">...&lt;/details>
&lt;details name="frame-characteristics" id="d2" open>...&lt;/details>
&lt;details name="frame-characteristics" id="d3">...&lt;/details>
&lt;/section></code></pre>

<p>because setting the <code data-x="attr-details-open">open</code> attribute on <code
data-x="">d2</code> removes it from <code data-x="">d1</code>.</p>

<p>The same happens when the user activates the <code>summary</code> element inside of <code
data-x="">d2</code>.</p>
</div>

<div class="example">

<p>Because the <code data-x="attr-details-open">open</code> attribute is added and removed
Expand Down Expand Up @@ -108868,8 +109098,17 @@ document.body.appendChild(frame)</code></pre>
<span>erase all event listeners and handlers</span> given <var>document</var>'s <span>relevant
global object</span>.</p></li>

<li>Let <var>oldFlag</var> be the value of <var>document</var>'s <span
data-x="concept-document-fire-mutation-events-flag">fire mutation events flag</span>.</li>

<li>Set <var>document</var>'s <span data-x="concept-document-fire-mutation-events-flag">fire
mutation events flag</span> to false.</li>

<li><p><span data-x="concept-node-replace-all">Replace all</span> with null within
<var>document</var>, without firing any mutation events.</p></li>
<var>document</var>.</p></li>

<li>Set <var>document</var>'s <span data-x="concept-document-fire-mutation-events-flag">fire
mutation events flag</span> to <var>oldFlag</var>.</li>

<li>
<p>If <var>document</var> is <span>fully active</span>, then:</p>
Expand Down Expand Up @@ -134232,6 +134471,7 @@ interface <dfn interface>External</dfn> {
<td><code>summary</code>*;
<span data-x="Flow content">flow</span></td>
<td><span data-x="global attributes">globals</span>;
<code data-x="attr-details-name">name</code>;
<code data-x="attr-details-open">open</code></td>
<td><code>HTMLDetailsElement</code></td>
</tr>
Expand Down Expand Up @@ -136503,6 +136743,11 @@ interface <dfn interface>External</dfn> {
<span data-x="attr-fe-name">form-associated custom elements</span>
<td> Name of the element to use for <span>form submission</span> and in the <code data-x="dom-form-elements">form.elements</code> API <!--or: Name of the element to use in the <code data-x="dom-form-elements">form.elements</code> API. -->
<td> <a href="#attribute-text">Text</a>*
<tr>
<th> <code data-x="">name</code>
<td> <code data-x="attr-details-name">details</code>
<td> Name of group of mutually-exclusive <code>details</code> elements
<td> <a href="#attribute-text">Text</a>*
<tr>
<th> <code data-x="">name</code>
<td> <code data-x="attr-form-name">form</code>
Expand Down