Add feature: Heading Nodes vs textContent

[danjacquemin]

Ran into an issue where I needed some SPAN content passed thru to the button text. Ex: a FontAwesome I tag.

In your ARIAaccordion.setupHeadingButton function the following changes did the trick.

// dhj; var buttonText;
var headingNodes;
...
// dhj; buttonText = heading.textContent;
headingNodes = heading.cloneNode(true); // clone the heading contents
// if you waited to delete the heading contents, you can skip the cloning
...
//dhj;  newButton.appendChild(doc.createTextNode(buttonText));
while( headingNodes.childNodes.length > 0 ) {    // loop thru the heading content
  newButton.appendChild(headingNodes.childNodes[0]);  // adding the nodes to the button
}

So, there ya go.
It might be something useful.

Share and enjoy.

dhj

[scottaohara]

Agree, that is useful and something I was going to tackle as part of #9

However, I'd be happy to accept a PR if you'd like to submit one per these changes :)

[danjacquemin]

I'll see what I can do.
Thanks!

[krokodok]

Will use that patch for now. Would be happy to see this in a future release!

[krokodok]

I had to add this code to the toggle function, so that arias get correctly updated:

if (e.target.nodeName != 'BUTTON') {
   thisTrigger = e.target.parentElement;
}

[scottaohara]

assuming your comment is in reference to the patch, @krokodok ?

[krokodok]

Yes, I used @danjacquemin 's code.

[smillart]

Sorry guys, this feature is very useful but sketchy regarding heading contents. According to W3C specifications for the button element, block-level elements are not allowed as child of an inline-level element.

Therefore, to remain compliant with the W3C specifications you need to check if heading contents are not block-level elements before injecting them as button childs.

[scottaohara]

headings and buttons both have a content model that allows for phrasing content as child elements, so your note to be cautious about this is appropriate, but not necessarily for the reason you state.

what would be more concerning is if authors had headings containing interactive elements and those were then attempted to be injected into a button.

tbh, i think the script needs to go in a different direction on this / among a few other areas. Not going to close this issue or the linked PR, in case anyone else wants to do something similar. but also not moving forward on this anytime soon either.

[smillart]

The div element is not an interactive element but could stand as heading content. However, as child of a button element... it's invalid HTML. Regarding @danjacquemin needs, few inline-level elements (e.g. span, i, small, time...) only should be allowed to be injected into the button element.

[scottaohara]

a div is an invalid child of a heading.

as i already stated, buttons and headings both allow for phrasing content as expected child elements. buttons have additional restrictions of disallowing interactive elements or elements with tabindex=0 on them.

Phrasing elements are listed here:
https://html.spec.whatwg.org/multipage/dom.html#phrasing-content-2

that said, the usefulness of injecting elements beyond the ability for people to include icons within their accordion trigger buttons is negligible since button elements are meant to treat child nodes as presentational.

[smillart]

Sorry... my mistake about div as child of a heading.

That said, common people don't know about HTML specifications. The active PR don't care about that and injects blindly.

I just wanted to draw your attention about W3C standards ... But I'm sure that it will be the case for the next releases ;)

[scottaohara]

I just wanted to draw your attention about W3C standards

thank you. i literally laughed out loud at this.