-
Notifications
You must be signed in to change notification settings - Fork 1.3k
wip: Use stacking contexts to determine non-inert elements outside modals #8796
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
import {mergeProps} from '@react-aria/utils'; | ||
import {OverlayTriggerState} from '@react-stately/overlays'; | ||
import {useEffect} from 'react'; | ||
|
@@ -58,7 +59,7 @@ export function useModalOverlay(props: AriaModalOverlayProps, state: OverlayTrig | |
|
||
useEffect(() => { | ||
if (state.isOpen && ref.current) { | ||
return ariaHideOutside([ref.current], {shouldUseInert: true}); | ||
return hideElementsBehind(ref.current); | ||
} | ||
}, [state.isOpen, ref]); | ||
|
||
|
@@ -67,3 +68,120 @@ export function useModalOverlay(props: AriaModalOverlayProps, state: OverlayTrig | |
underlayProps | ||
}; | ||
} | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 rootStackingContext = roots.find(r => r.contains(element)) || document.documentElement; | ||
let elementZIndex = getZIndex(rootStackingContext); | ||
|
||
return ariaHideOutside([element], { | ||
shouldUseInert: true, | ||
getVisibleNodes: el => { | ||
let node: Element | null = el; | ||
let ancestors: Element[] = []; | ||
while (node && node !== root) { | ||
ancestors.unshift(node); | ||
node = node.parentElement; | ||
} | ||
|
||
// If an ancestor element of the added target is a stacking context root, | ||
// use that to determine if the element should be preserved. | ||
let stackingContext = ancestors.find(el => isStackingContext(el)); | ||
if (stackingContext) { | ||
if (shouldPreserve(element, elementZIndex, stackingContext)) { | ||
return [el]; | ||
} | ||
return []; | ||
} else { | ||
// Otherwise, find stacking context roots within the added element, and compare with the modal element. | ||
let roots = getStackingContextRoots(el); | ||
let preservedElements: Element[] = []; | ||
for (let root of roots) { | ||
if (shouldPreserve(element, elementZIndex, root)) { | ||
preservedElements.push(root); | ||
} | ||
} | ||
return preservedElements; | ||
} | ||
} | ||
}); | ||
} | ||
|
||
function shouldPreserve(baseElement: Element, baseZIndex: number, element: Element) { | ||
if (baseElement.contains(element)) { | ||
return true; | ||
} | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this is where the check for "float" would come in |
||
if (baseElement.compareDocumentPosition(element) & Node.DOCUMENT_POSITION_FOLLOWING) { | ||
return true; | ||
} | ||
} else if (zIndex > baseZIndex) { | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
return ( | ||
el === document.documentElement || | ||
(style.position !== 'static' && style.zIndex !== 'auto') || | ||
('containerType' in style && style.containerType.includes('size')) || | ||
(style.zIndex !== 'auto' && isFlexOrGridItem(el)) || | ||
parseFloat(style.opacity) < 1 || | ||
('mixBlendMode' in style && style.mixBlendMode !== 'normal') || | ||
('transform' in style && style.transform !== 'none') || | ||
('webkitTransform' in style && style.webkitTransform !== 'none') || | ||
('scale' in style && style.scale !== 'none') || | ||
('rotate' in style && style.rotate !== 'none') || | ||
('translate' in style && style.translate !== 'none') || | ||
('filter' in style && style.filter !== 'none') || | ||
('webkitFilter' in style && style.webkitFilter !== 'none') || | ||
('backdropFilter' in style && style.backdropFilter !== 'none') || | ||
('perspective' in style && style.perspective !== 'none') || | ||
('clipPath' in style && style.clipPath !== 'none') || | ||
('mask' in style && style.mask !== 'none') || | ||
('maskImage' in style && style.maskImage !== 'none') || | ||
('maskBorder' in style && style.maskBorder !== 'none') || | ||
style.isolation === 'isolate' || | ||
/position|z-index|opacity|mix-blend-mode|transform|webkit-transform|scale|rotate|translate|filter|webkit-filter|backdrop-filter|perspective|clip-path|mask|mask-image|mask-border|isolation/.test(style.willChange) || | ||
/layout|paint|strict|content/.test(style.contain) | ||
); | ||
} | ||
|
||
function getStackingContextRoots(root: Element = document.body) { | ||
let roots: Element[] = []; | ||
|
||
function walk(el: Element) { | ||
if (!isElementVisible(el)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we want to use 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? |
||
return; | ||
} | ||
|
||
if (isStackingContext(el)) { | ||
roots.push(el); | ||
} else { | ||
for (const child of el.children) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will this run on |
||
walk(child); | ||
} | ||
} | ||
} | ||
|
||
walk(root); | ||
return roots; | ||
} | ||
|
||
function getZIndex(element: Element) { | ||
return Number(getComputedStyle(element).zIndex) || 0; | ||
} | ||
|
||
function isFlexOrGridItem(element: Element) { | ||
let parent = element.parentElement; | ||
return parent && /flex|grid/.test(getComputedStyle(parent).display); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ | |
import {DOMAttributes, RefObject} from '@react-types/shared'; | ||
import {isElementInChildOfActiveScope} from '@react-aria/focus'; | ||
import {useEffect} from 'react'; | ||
import {useFocusWithin, useInteractOutside} from '@react-aria/interactions'; | ||
import {useFocusWithin} from '@react-aria/interactions'; | ||
|
||
export interface AriaOverlayProps { | ||
/** Whether the overlay is currently open. */ | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
let {focusWithinProps} = useFocusWithin({ | ||
isDisabled: !shouldCloseOnBlur, | ||
|
@@ -147,6 +147,9 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject<Element | nul | |
// fixes a firefox issue that starts text selection https://bugzilla.mozilla.org/show_bug.cgi?id=1675846 | ||
if (e.target === e.currentTarget) { | ||
e.preventDefault(); | ||
if (isDismissable) { | ||
onInteractOutsideStart(e); | ||
} | ||
} | ||
}; | ||
|
||
|
@@ -156,7 +159,12 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject<Element | nul | |
...focusWithinProps | ||
}, | ||
underlayProps: { | ||
onPointerDown: onPointerDownUnderlay | ||
onPointerDown: onPointerDownUnderlay, | ||
onClick(e) { | ||
if (isDismissable && isOpen && e.target === e.currentTarget) { | ||
onInteractOutside(e.nativeEvent as PointerEvent); | ||
} | ||
} | ||
} | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to aria/utils eventually, or depending on RFC, i guess this won't matter, we'll have access