Update type for document option to allow for shadowRoot elements. about focus-trap HOT 7 CLOSED

@driskull Thanks for pointing this out! There does seem to be a discrepancy there with the typings since Shadow DOM support was added.

However, the typings for the document property only allow for the Document. Should this type be expanded to include any HTMLElement?

Not quite. The code expects to find an activeElement property on that object, so it can't be any random HTMLElement since activeElement is not part of that interface.

The only other option would be a ShadowRoot, however I think this piece of the documentation is not correct.

That comment/bullet point should probably be removed. I think that's the right fix here.

This commit added that bullet point about setting the document option, however, we have 2 Shadow DOM examples and neither of them need to be setting the document option to make them work (and their demos don't set that option as-is):

• Inside open shadow dom
• With shadow dom

I think I may have written that while I was still iterating through that PR (#516) and ultimately found another solution with composedPath and then forgot to remove that bit.

Even my commit for that PR, which introduced the "Inside open shadow dom" demo, didn't actually set the document option in the end.

What do you think? Do you have a new use case where you actually think you need to set this option to your Shadow Root? I'm not sure how the trap will behave in that case. My guess is that this is when you'll end-up with a partially-trapped focus case like the Working inside an iframe demo where focus is trapped within the iframe, but you're still free to click on anything outside the iframe.

As part of fixing this bug, I think it would be nice to relocate the link to the "Inside an iframe" demo to the document option docs like this (the link doesn't exist today):

- **document** {Document}: Default: `window.document`. Document where the
  focus trap will be active. This allows to use FocusTrap
  [in an iFrame context](https://focus-trap.github.io/focus-trap/#demo-in-iframe).

from focus-trap.

Thanks @stefcameron, I was running into an some issues where the focus trap was preventing focus of an element that was slotted into a shadow DOM element. I'll try the code from the "with shadow dom" example. Specifically this part...

tabbableOptions: {
      getShadowRoot(node) {
        if (node === closedShadowHostEl) {
          return closedShadowEl;
        }
      },
    },

We are currently setting the following:

tabbableOptions = {
  getShadowRoot: true
};

Our specific use case is a open shadow DOM modal component. However, we noticed sometimes the first click on another dropdown component slotted within the modal doesn't do anything but the second click works. Setting the document option to the component host seems to have resolved that.

from focus-trap.

@driskull Interesting! Please let me know what you find out when you can.

from focus-trap.

Ok. I don't think it has to do anything with shadowDOM. It seems like the "autocomplete" menu updates once a user starts typing and focus-trap doesn't know about the new elements.

I tried calling updateContainerElements(myFocusTrapEl) but that didnt seem to work.

If I deactivate the focus trap and then activate it it does work.

Would it be best to have a mutation listener to deactivate and reactivate the focus trap if any elements are added to the focus trap after initialization? Or should updateContainerElements work here?

from focus-trap.

Would it be best to have a mutation listener to deactivate and reactivate the focus trap if any elements are added to the focus trap after initialization? Or should updateContainerElements work here?

Calling updateContainerElements(trapEl) should essentially cause the trap to run tabbable(trapEl) internally to update its notion of tabbable nodes in every one of the trap's containers (sounds like just one, in your case). That list of tabbable nodes is then used whenever a focus-related pointer or keyboard event occurs (one of the ones monitored on the document) to determine if the click was contained within the trap or not, and prevent it if not.

It comes down to checks using findContainerIndex()

A mutation observer might work, but it seems like if updateContainerElements() didn't work, it's because it may have executed too early and the slotted element wasn't yet in the slot (like an appearing/disappearing "auto-complete" menu).

from focus-trap.

Thanks for your help @stefcameron.

The issue I'm seeing is when I call updateContainerElements(myEl) with the same element as I created the focus trap with it doesn't seem to update the container elements but if I call it with a parent of the element used to create the focus trap then it does seem to work.

I'll do some more digging. Thanks for your help.

from focus-trap.

I'll do some more digging. Thanks for your help.

My pleasure! I hope you find a solution. In the meantime, I'm going to remove that piece of the documentation that isn't correct.

from focus-trap.