wip: Use stacking contexts to determine non-inert elements outside modals #8796
Conversation
devongovett
force-pushed
the
modal-inert-stacking
branch
from
96a2fbe to
564cc88
Compare
| @@ -119,7 +119,7 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject<Element | nul | |||
| }; | |||
|
|
|||
| // Handle clicking outside the overlay to close it | |||
| useInteractOutside({ref, onInteractOutside: isDismissable && isOpen ? onInteractOutside : undefined, onInteractOutsideStart}); | |||
| // useInteractOutside({ref, onInteractOutside: isDismissable && isOpen ? onInteractOutside : undefined, onInteractOutsideStart}); | |||
There was a problem hiding this comment.
Using clicking the underlay as interacting outside since this avoids closing when clicking on an element that's on top of it. Will need some backward compatibility logic here since I think underlayProps isn't always used.
| function isStackingContext(el: Element) { | ||
| let style = getComputedStyle(el); | ||
|
|
||
| // https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Stacking_context#features_creating_stacking_contexts |
There was a problem hiding this comment.
probably have to also account for floating elements https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Stacking_floating_elements
today I learned that getComputedStyle will return a transform matrix for any translates, include translate3d
There was a problem hiding this comment.
I assume a shadow root starts a new stacking context? I can't find good information on the topic, will try it if i have time
| @@ -13,6 +13,7 @@ | |||
| import {ariaHideOutside} from './ariaHideOutside'; | |||
| import {AriaOverlayProps, useOverlay} from './useOverlay'; | |||
| import {DOMAttributes, RefObject} from '@react-types/shared'; | |||
| import {isElementVisible} from '../../utils/src/isElementVisible'; | |||
There was a problem hiding this comment.
move to aria/utils eventually, or depending on RFC, i guess this won't matter, we'll have access
|
|
||
| let zIndex = getZIndex(element); | ||
| if (zIndex === baseZIndex) { | ||
| // If two elements have the same z-index, compare their document order. |
There was a problem hiding this comment.
i think this is where the check for "float" would come in
|
|
||
| function hideElementsBehind(element: Element, root = document.body) { | ||
| // TODO: automatically determine root based on parent stacking context of element? | ||
| let roots = getStackingContextRoots(root); |
There was a problem hiding this comment.
this could be potentially really bad, calling getComputedStyle on every element on the entire page, though unlikely
I guess it only happens on mount of a dialog or something along those lines
We can't really keep a single, on page load data structure with all of them and update as the dom updates because css could change, and it'd involve mutation observers on way too many things which would cause a performance hit for other reasons
it'd be nice if we could just compare two elements traversing upwards instead, but that'd be really hard for the initial hide
| let roots: Element[] = []; | ||
|
|
||
| function walk(el: Element) { | ||
| if (!isElementVisible(el)) { |
There was a problem hiding this comment.
I don't think we want to use isElementVisible with the visibilityProperty: true and opacityProperty: false it currently has hard set
We might run into a problem if something is faded into the foreground where those properties start as hidden discrete and opacity 0
Instead, I think it should count both of those as visible for the purposes of these checks
What do you think?
| if (isStackingContext(el)) { | ||
| roots.push(el); | ||
| } else { | ||
| for (const child of el.children) { |
There was a problem hiding this comment.
will this run on script and style tags? can we skip any of those?
RSP Component Milestones (view)
Related issues:
Currently, our dialogs mark all elements outside them as
inert. This makes it difficult to use them with third party components that use portals, such as external dialogs, menus, browser extensions, embedded widgets like cookie consents, toasts, support chats, etc. Even before we switched toinertfromaria-hidden, FocusScope would steal focus back to the dialog and screen readers could not access these elements. We havedata-react-aria-top-layeras an opt out, but you must manually apply it which can be difficult.This PR is an attempt to make this work automatically. Instead of marking all elements outside dialogs as inert (with a few exceptions), it compares their z-indexes with the modal. Any elements that are visually above the modal get preserved. This works by traversing down from the document body to find the root stacking contexts, and then comparing these with the root stacking context of the modal itself. When a new element such as a toast comes in after the dialog opens the same process occurs.
Since we use inert, we don't need FocusScope to contain focus for the most part. This will also mean the browser will handle tabbing, which should fix some issues with shadow DOM and native elements. Some exceptions to handle still:
Perhaps we could do a more limited focus trap just within the window/iframe instead of only within the dialog. That would allow tabbing to other elements on top of the dialog, but not out of the window.
To do