Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Dialog, Popover, Menu, Select, PreviewCard] Unify backdrop implementation #841

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

[Dialog, Popover, Menu, Select, PreviewCard] Unify backdrop implementation #841

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
cabfa29
[Dialog] Use internal backdrop element
michaldudak Nov 19, 2024
3677f9f
Simplify and unify backdrops
michaldudak Nov 20, 2024
432d931
docs
michaldudak Nov 20, 2024
32eef9c
Merge branch 'master' into internal-backdrop
michaldudak Nov 21, 2024
c770422
Simplify proptypes
michaldudak Nov 21, 2024
73db7f2
Merge remote-tracking branch 'upstream/master' into internal-backdrop
michaldudak Nov 22, 2024
97d68fb
Add MenuBackdrop
michaldudak Nov 22, 2024
dd2b26e
Merge branch 'master' into internal-backdrop
michaldudak Nov 25, 2024
c15dbb1
Merge remote-tracking branch 'upstream/master' into internal-backdrop
michaldudak Nov 27, 2024
008f10e
Change directory to lowercase
michaldudak Nov 27, 2024
bb01c81
Merge remote-tracking branch 'upstream/master' into internal-backdrop
michaldudak Nov 27, 2024
7f6be6a
Fix casing mess
michaldudak Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/data/api/alert-dialog-backdrop.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
{
"props": {
"className": { "type": { "name": "union", "description": "func<br>&#124;&nbsp;string" } },
"container": {
"type": { "name": "union", "description": "HTML element<br>&#124;&nbsp;func" },
"default": "false"
},
"keepMounted": { "type": { "name": "bool" }, "default": "false" },
"render": { "type": { "name": "union", "description": "element<br>&#124;&nbsp;func" } }
},
Expand Down
4 changes: 4 additions & 0 deletions docs/data/api/dialog-backdrop.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
{
"props": {
"className": { "type": { "name": "union", "description": "func<br>&#124;&nbsp;string" } },
"container": {
"type": { "name": "union", "description": "HTML element<br>&#124;&nbsp;func" },
"default": "false"
},
"keepMounted": { "type": { "name": "bool" }, "default": "false" },
"render": { "type": { "name": "union", "description": "element<br>&#124;&nbsp;func" } }
},
Expand Down
1 change: 1 addition & 0 deletions docs/data/translations/api-docs/alert-dialog-backdrop/alert-dialog-backdrop.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"className": {
"description": "Class names applied to the element or a function that returns them based on the component&#39;s state."
},
"container": { "description": "The container element to which the backdrop is appended to." },
"keepMounted": {
"description": "If <code>true</code>, the backdrop element is kept in the DOM when closed."
},
Expand Down
1 change: 1 addition & 0 deletions docs/data/translations/api-docs/dialog-backdrop/dialog-backdrop.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"className": {
"description": "Class names applied to the element or a function that returns them based on the component&#39;s state."
},
"container": { "description": "The container element to which the backdrop is appended to." },
"keepMounted": {
"description": "If <code>true</code>, the backdrop element is kept in the DOM when closed."
},
Expand Down
5 changes: 5 additions & 0 deletions docs/reference/generated/alert-dialog-backdrop.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
"type": "string | (state) => string",
"description": "Class names applied to the element or a function that returns them based on the component's state."
},
"container": {
"type": "React.Ref | HTMLElement | null",
"default": "false",
"description": "The container element to which the backdrop is appended to."
},
"keepMounted": {
"type": "boolean",
"default": "false",
Expand Down
5 changes: 5 additions & 0 deletions docs/reference/generated/dialog-backdrop.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
"type": "string | (state) => string",
"description": "Class names applied to the element or a function that returns them based on the component's state."
},
"container": {
"type": "React.Ref | HTMLElement | null",
"default": "false",
"description": "The container element to which the backdrop is appended to."
},
"keepMounted": {
"type": "boolean",
"default": "false",
Expand Down
46 changes: 28 additions & 18 deletions packages/react/src/AlertDialog/Backdrop/AlertDialogBackdrop.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import * as React from 'react';
import PropTypes from 'prop-types';
import { FloatingPortal } from '@floating-ui/react';
import { useAlertDialogRootContext } from '../Root/AlertDialogRootContext';
import { useDialogBackdrop } from '../../Dialog/Backdrop/useDialogBackdrop';
import { useComponentRenderer } from '../../utils/useComponentRenderer';
import type { TransitionStatus } from '../../utils/useTransitionStatus';
import type { BaseUIComponentProps } from '../../utils/types';
import type { CustomStyleHookMapping } from '../../utils/getStyleHookProps';
import { popupOpenStateMapping as baseMapping } from '../../utils/popupOpenStateMapping';
import { HTMLElementType } from '../../utils/proptypes';

const customStyleHookMapping: CustomStyleHookMapping<AlertDialogBackdrop.OwnerState> = {
...baseMapping,
Expand Down Expand Up @@ -37,32 +37,29 @@ const AlertDialogBackdrop = React.forwardRef(function AlertDialogBackdrop(
props: AlertDialogBackdrop.Props,
forwardedRef: React.ForwardedRef<HTMLDivElement>,
) {
const { render, className, keepMounted = false, ...other } = props;
const { open, hasParentDialog, animated } = useAlertDialogRootContext();
const { render, className, keepMounted = false, container, ...other } = props;
const { open, hasParentDialog, mounted, transitionStatus } = useAlertDialogRootContext();

const { getRootProps, mounted, transitionStatus } = useDialogBackdrop({
animated,
open,
ref: forwardedRef,
});

const ownerState: AlertDialogBackdrop.OwnerState = { open, transitionStatus };
const ownerState: AlertDialogBackdrop.OwnerState = React.useMemo(
() => ({
open,
transitionStatus,
}),
[open, transitionStatus],
);

const { renderElement } = useComponentRenderer({
render: render ?? 'div',
className,
ownerState,
propGetter: getRootProps,
extraProps: other,
ref: forwardedRef,
extraProps: { role: 'presentation', hidden: !mounted, ...other },
customStyleHookMapping,
});

if (!mounted && !keepMounted) {
return null;
}

if (hasParentDialog) {
// no need to render nested backdrops
// no need to render nested backdrops
const shouldRender = (keepMounted || mounted) && !hasParentDialog;
if (!shouldRender) {
return null;
}

Expand All @@ -77,6 +74,11 @@ namespace AlertDialogBackdrop {
* @default false
*/
keepMounted?: boolean;
/**
* The container element to which the backdrop is appended to.
* @default false
*/
container?: HTMLElement | null | React.MutableRefObject<HTMLElement | null>;
}

export interface OwnerState {
Expand All @@ -98,6 +100,14 @@ AlertDialogBackdrop.propTypes /* remove-proptypes */ = {
* Class names applied to the element or a function that returns them based on the component's state.
*/
className: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
/**
* The container element to which the backdrop is appended to.
* @default false
*/
container: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([
HTMLElementType,
PropTypes.func,
]),
/**
* If `true`, the backdrop element is kept in the DOM when closed.
*
Expand Down
3 changes: 2 additions & 1 deletion packages/react/src/AlertDialog/Popup/AlertDialogPopup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client';
import * as React from 'react';
import PropTypes from 'prop-types';
import { FloatingFocusManager, FloatingPortal } from '@floating-ui/react';
import { FloatingFocusManager, FloatingOverlay, FloatingPortal } from '@floating-ui/react';
import { useDialogPopup } from '../../Dialog/Popup/useDialogPopup';
import { useAlertDialogRootContext } from '../Root/AlertDialogRootContext';
import { useComponentRenderer } from '../../utils/useComponentRenderer';
Expand Down Expand Up @@ -115,6 +115,7 @@ const AlertDialogPopup = React.forwardRef(function AlertDialogPopup(

return (
<FloatingPortal root={container}>
{mounted && <FloatingOverlay />}
<FloatingFocusManager
context={floatingContext}
modal
Expand Down
45 changes: 26 additions & 19 deletions packages/react/src/Dialog/Backdrop/DialogBackdrop.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import { FloatingPortal } from '@floating-ui/react';
import { useDialogBackdrop } from './useDialogBackdrop';
import { useDialogRootContext } from '../Root/DialogRootContext';
import { useComponentRenderer } from '../../utils/useComponentRenderer';
import { type TransitionStatus } from '../../utils/useTransitionStatus';
import { type BaseUIComponentProps } from '../../utils/types';
import { type CustomStyleHookMapping } from '../../utils/getStyleHookProps';
import { popupOpenStateMapping as baseMapping } from '../../utils/popupOpenStateMapping';
import { HTMLElementType } from '../../utils/proptypes';

const customStyleHookMapping: CustomStyleHookMapping<DialogBackdrop.OwnerState> = {
...baseMapping,
Expand Down Expand Up @@ -38,39 +38,33 @@ const DialogBackdrop = React.forwardRef(function DialogBackdrop(
props: DialogBackdrop.Props,
forwardedRef: React.ForwardedRef<HTMLDivElement>,
) {
const { render, className, keepMounted = false, ...other } = props;
const { open, hasParentDialog, animated } = useDialogRootContext();

const { getRootProps, mounted, transitionStatus } = useDialogBackdrop({
animated,
open,
ref: forwardedRef,
});
const { render, className, keepMounted = false, container, ...other } = props;
const { open, hasParentDialog, mounted, transitionStatus } = useDialogRootContext();

const ownerState: DialogBackdrop.OwnerState = React.useMemo(
() => ({ open, transitionStatus }),
() => ({
open,
transitionStatus,
}),
[open, transitionStatus],
);

const { renderElement } = useComponentRenderer({
render: render ?? 'div',
className,
ownerState,
propGetter: getRootProps,
extraProps: other,
ref: forwardedRef,
extraProps: { role: 'presentation', hidden: !mounted, ...other },
customStyleHookMapping,
});

if (!mounted && !keepMounted) {
return null;
}

if (hasParentDialog) {
// no need to render nested backdrops
// no need to render nested backdrops
const shouldRender = (keepMounted || mounted) && !hasParentDialog;
if (!shouldRender) {
return null;
}

return <FloatingPortal>{renderElement()}</FloatingPortal>;
return <FloatingPortal root={container}>{renderElement()}</FloatingPortal>;
});

namespace DialogBackdrop {
Expand All @@ -81,6 +75,11 @@ namespace DialogBackdrop {
* @default false
*/
keepMounted?: boolean;
/**
* The container element to which the backdrop is appended to.
* @default false
*/
container?: HTMLElement | null | React.MutableRefObject<HTMLElement | null>;
}

export interface OwnerState {
Expand All @@ -102,6 +101,14 @@ DialogBackdrop.propTypes /* remove-proptypes */ = {
* Class names applied to the element or a function that returns them based on the component's state.
*/
className: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
/**
* The container element to which the backdrop is appended to.
* @default false
*/
container: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([
HTMLElementType,
PropTypes.func,
]),
/**
* If `true`, the backdrop element is kept in the DOM when closed.
*
Expand Down
70 changes: 0 additions & 70 deletions packages/react/src/Dialog/Backdrop/useDialogBackdrop.ts
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion packages/react/src/Dialog/Popup/DialogPopup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client';
import * as React from 'react';
import PropTypes from 'prop-types';
import { FloatingFocusManager, FloatingPortal } from '@floating-ui/react';
import { FloatingFocusManager, FloatingOverlay, FloatingPortal } from '@floating-ui/react';
import { useDialogPopup } from './useDialogPopup';
import { useDialogRootContext } from '../Root/DialogRootContext';
import { useComponentRenderer } from '../../utils/useComponentRenderer';
Expand Down Expand Up @@ -114,6 +114,7 @@ const DialogPopup = React.forwardRef(function DialogPopup(

return (
<FloatingPortal root={container}>
{modal && mounted && <FloatingOverlay />}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about z-index?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, right, this is going to be tricky. We could figure out the z-index of the popup and set this one lower, but this will be unreliable in some cases. We might have to expose the className of this part, or the whole part itself, to be consistent with the rest of the API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this another problem solved by a separate Portal part? 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, unless I'm missing something. Having an explicit Portal wouldn't help determining the right z-index of the internal backdrop.
Perhaps a better solution is not rendering the internal backdrop, but adding the inert attribute to all elements except the popup?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a better solution is not rendering the internal backdrop, but adding the inert attribute to all elements except the popup?

This could work. guards={false} on FloatingFocusManager does this, but it allows escaping to the browser address bar of course, which we didn't want to allow. Guess this is relevant: floating-ui/floating-ui#3098

The other option is to get the computed z-index style of the Popup and apply the z-index to be just one less on FloatingOverlay?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could work. guards={false} on FloatingFocusManager does this, but it allows escaping to the browser address bar of course, which we didn't want to allow.

I'll take a look at Floating UI implementation and see what I can do, then.

The other option is to get the computed z-index style of the Popup and apply the z-index to be just one less on FloatingOverlay?

This was my first idea, but I realized someone might style a child of the popup for some reason. Or the z-index could change depending on how many dialogs are open, so this might not be the most reliable solution.

<FloatingFocusManager
context={floatingContext}
modal={modal}
Expand Down
11 changes: 3 additions & 8 deletions packages/react/src/Popover/Backdrop/PopoverBackdrop.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { FloatingPortal } from '@floating-ui/react';
import { usePopoverRootContext } from '../Root/PopoverRootContext';
import { useComponentRenderer } from '../../utils/useComponentRenderer';
import { HTMLElementType } from '../../utils/proptypes';
import { usePopoverBackdrop } from './usePopoverBackdrop';
import type { BaseUIComponentProps } from '../../utils/types';
import { type CustomStyleHookMapping } from '../../utils/getStyleHookProps';
import { popupOpenStateMapping as baseMapping } from '../../utils/popupOpenStateMapping';
Expand Down Expand Up @@ -39,13 +38,10 @@ const PopoverBackdrop = React.forwardRef(function PopoverBackdrop(
props: PopoverBackdrop.Props,
forwardedRef: React.ForwardedRef<HTMLDivElement>,
) {
const { className, render, keepMounted = false, container, ...otherProps } = props;

const { className, render, keepMounted = false, container, ...other } = props;
const { open, mounted, transitionStatus } = usePopoverRootContext();

const { getBackdropProps } = usePopoverBackdrop();

const ownerState = React.useMemo(
const ownerState: PopoverBackdrop.OwnerState = React.useMemo(
() => ({
open,
transitionStatus,
Expand All @@ -54,12 +50,11 @@ const PopoverBackdrop = React.forwardRef(function PopoverBackdrop(
);

const { renderElement } = useComponentRenderer({
propGetter: getBackdropProps,
render: render ?? 'div',
className,
ownerState,
ref: forwardedRef,
extraProps: otherProps,
extraProps: { role: 'presentation', hidden: !mounted, ...other },
customStyleHookMapping,
});

Expand Down
Loading