FocusScope not working when used inside shadowRoot

[sventschui]

🐛 Bug Report

When used inside a shadowRoot the FocusScope currently has some issues:

• Focused element will not be determined correctly as document.activeElement will refer to the parent node of the shadowRoot the currently focused element is in. The activeElement needs to be determined recursively traversing (open) shadowRoots (document.activeElement.shadowRoot.activeElement.shadowRoot.activeElement......). This leads to invalid detection whether an element is in scope and also restores focus wrongly.
• e.target in focus/blur events is referring to the parent node of the shadowRoot of the focused element. e.composedPath()[0] can be used to determine the actually focused element inside the custom element. ('composedPath' in e ? e.composedPath()[0] : e.target)
• Chrome: TypeError: Cannot read property 'focus' of null in the onBlur of useFocusContainment due to e.target being null inside the rAF (not sure this is really related to custom element / shadowRoot or a general bug
• Safari: Focus is looped through focusable elements

I patched this in our libs using patch-package so I'm happy to file a PR if you'd like to support custom elements. I'm not sure how this affects other packages of the spectrum ecosystem and thus wanted to check in prior to opening a PR.

🤔 Expected Behavior

FocusRing can be used with custom elements / shadowRoots

😯 Current Behavior

See Bug Report above

💁 Possible Solution

• Instead of document.activeElement use activeElement()

function activeElement() {
    let activeElement = document.activeElement;
    while (activeElement && activeElement.shadowRoot && activeElement.shadowRoot.activeElement) {
        activeElement = activeElement.shadowRoot.activeElement;
    }
    return activeElement;
}

• Instead of e.target use 'composedPath' in e ? e.composedPath()[0] : e.target

🔦 Context

In our Microfrontend Framework we encapsulate modules inside custom elements. To escape them we use a portal-root custom element in the body where all modals will be rendered into. Inside this we use FocusScope to contain focus.

💻 Code Sample

https://codesandbox.io/s/vigilant-hofstadter-3wf4i?file=/src/index.js

🌍 Your Environment

Software	Version(s)
@react-aria/focus	3.2.3
Browser	Chrome 87.0.4280.141 / Safari 14.0.2
Operating System	Mac OS 10.15

🧢 Your Company/Team

Undisclosed

🕷 Tracking Issue (optional)

n/a

[devongovett]

Shadow DOM support is definitely something we're interested in. I'm sure quite a few things are broken at the moment beyond just FocusScope. If you're interested in contributing, that would be amazing!

A few other places I can think of that will need work:

• Any other place we use document.activeElement
• Places where we use Node#contains, querySelector, TreeWalker, or other DOM traversal functions
• event.target usages

There are probably others as well.

In general, is it safe to assume that we can traverse into a shadow root though? What about closed shadow roots? Is it ok to not support those you think? Or assume that the closed shadow roots will forward focus internally as necessary, for example?

[nouman91]

I love this project, I will contribute to adding support for shadow DOM. Any pointers on what should I keep in mind while working on it?

[snowystinger]

Thank you! I think just the things @devongovett mentioned in the previous comment to start. Otherwise, make sure to check out the contribution page on our docs site https://react-spectrum.adobe.com/contribute.html

[nouman91]

Ok sure, I will check the contribution guide and work on the initial RFC.

[padmkris123]

I would love to use this fix as well. I too need this for implementing micro-frontends in our application. Is there any pending work that needs an extra helping hand?

[snowystinger]

If you want to have a look at it, that'd be a great help! We don't have any current work in the team going on for it and I don't think we've heard from the other people in this thread in a long while.

[snowystinger]

@theomessin
Sorry for the delay; we've been in release prep mode. During our grooming session, we discussed this PR and what we're hoping for in the future of shadowDOM support. The team is a bit worried; we think this needs to be considered as a general approach for the whole library, and it is an area where we lack expertise. We'd be looking for an overview of what needs to be done to support ShadowDOM so that we aren't just reactive. For instance, this PR has done a good job of fixing one particular issue. However, a few lines up is another call to element.contains

react-spectrum/packages/@react-aria/interactions/src/useInteractOutside.ts

Line 116 in aa52464

if (!ownerDocument || !ownerDocument.documentElement.contains(event.target)) {

which we think would lead to Toasts and the top layer not working.

If this is something you would be interested in, we would be open to discussing how you think a shadow dom solution in React Aria would work before committing any individual patches.

[josephmarkus]

I ran into the same issue where keyboard focus doesn't work when an app that uses { useFocusRing } from "@react-aria/focus" is rendered in ShadowDOM

[joealden]

@snowystinger I know that the scope of react-aria is much larger than https://github.com/downshift-js/downshift, but their approach to this is an environment prop, which allows the caller to pass their own values/functions to be used. For reference, here is their type definition at the time of writing:

https://github.com/downshift-js/downshift/blob/a5d6310281e50e4fe91037de77f99d67955e7700/typings/index.d.ts#L83-L88

I'm wondering if a similar approach could be followed, but maybe using context instead (similar to RouterProvider), as that way it wouldn't have to be passed to individual hooks/components, and likely provides a better DX for your team (as a default "environment" could be the default context value, and use that instead of any direct window or document calls).

Just an idea - I don't know my way around this codebase, so maybe there are problems with following a similar pattern?

[ritz078]

@snowystinger Shouldn't this be easier now that we already support dynamic iframe starting next release ? I guess the change needed will be mostly in getOwnerDocument/getOwnerWindow?