Handle shadow children when trapping focus

[mxmason]

Summary

This overhaul of getFocusableChildren replaces querySelectorAll with a manual, recursive DOM traversal. This traversal is necessary because there's no way to query into shadow subtrees.

I have ideas for tests, as well, but I will follow up with them in a separate PR to keep this one small.

Huge thanks to the inimitable @alice for listening to me complain about Shadow DOM and helping me figure this algorithm out.

Relevant issues

Closes #322.

[KittyGiraudel]

Yaaaay! So excited about this. I asked some questions and left some comments. It‘s certainly a bit more heavy than I hoped for, but I guess it’s worth it, in order to account for shadow DOM trees, which are more and more common.

[KittyGiraudel]

Pretty sure this should be enough:

Suggested change

	if (!!curr.shadowRoot) {
	if (curr.shadowRoot) {

[mxmason]

I know I had a reason for wanting to write it this way, but the naïve tests I just did didn't yield any weird behavior.

[KittyGiraudel]

We could also write it like this, although it’s worth noting it would fail if there are over 65535 nodes (the typical maximum amount of arguments allowed by a function call):

Suggested change

	focusableEls = [...focusableEls, ...getFocusableChildren(curr.shadowRoot)]
	focusableEls.push(...getFocusableChildren(curr.shadowRoot))

If we do use .push(), we should also use it below. :)

[mxmason]

Hmm. I did a benchmark for this. On my machine, it looks like:

• In Chrome/Edge, [...oldNodes, ...newNodes] is fastest
• In Firefox and Safari, oldNodes.push.apply(oldNodes, newNodes) is fastest
• In all browsers, oldNodes.push(...newNodes) is slowest.

In Chromium, push.apply is second-fastest so I think it's the optimal choice between all 3 major browser engines. Interesting, because I would have thought it might be less expensive to make a new array than to modify one in place :)

[KittyGiraudel]

Fair enough!

[mxmason]

Amending my earlier commentary: I don't think this implementation detail will have that much impact on our users. The time it takes to do this operation is probably negligible in the real world, regardless of implementation. I'm happy to male the change we discussed, if you feel strongly about it!

[KittyGiraudel]

Definitely marginal if anything. :)

[KittyGiraudel]

What is a slot? 😅

[mxmason]

Slots allow authors to define a place (a slot) for developer-supplied Light DOM content to be rendered inside a Web Component. Let's take this template for the <fancy-card> Web Component:

<template id="fancy-card-template">
  <slot name="title">
    <!-- Developer-supplied Light DOM node goes here -->
  </slot>
  <slot name="content">
    <!-- Developer-supplied Light DOM node goes here -->
  </slot>
</template>

This template has two slots:

1. title, for the title of the card
2. content, for long-form text content

The developer might use it like this.

<fancy-card>
  <h2 slot="title">Why my cat is a weird little guy</h2>
  <p slot="content">He sits like a man.</p>
</fancy-card>

The <h2> will go inside the title slot, and the <p> will go inside the content slot. These slotted children will be in the Light DOM, which is why we have to specifically look for them by calling assignedElements() on the slot.

[KittyGiraudel]

Ah! Thank you for the explanation. :)

[KittyGiraudel]

Is that relevant to finding focusable children though?

[mxmason]

IMO, yes. It’s a common enough pattern for anchors and buttons to have child elements, and

1. we may as well bail out instead of walking into their subtrees, plus,
2. if the author has for some reason done something like a > a, the browser won’t count the descendant a as tabbable and nor should we

[KittyGiraudel]

It looks good to me (for what I understand of the shadow DOM). I would love if we could have some tests though, so we make sure not to break that behavior in the future. :)

Do you need support with Cypress or are you familiar with it?

[mxmason]

I’m happy to write the tests, @KittyGiraudel! I’ll do those once this is merged. After the tests, I have an idea to optimize the DOM tree walking algorithm I’ve done here, and the tests can make sure everything remains good 😃