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.

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

[Dialog] Support non-nested modal dialogs #1320

Open
aarongarciah opened this issue Jan 10, 2025 · 1 comment · May be fixed by #1327
Open

[Dialog] Support non-nested modal dialogs #1320

aarongarciah opened this issue Jan 10, 2025 · 1 comment · May be fixed by #1327
Assignees
Labels
component: dialog This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@aarongarciah
Copy link
Member

aarongarciah commented Jan 10, 2025

Bug report

Current behavior

Rendering several non-nested modal dialogs simultaneously causes issues. See the attached video and demo.

  1. When clicking inside a dialog, all opened dialogs close except the one you clicked on. This only happens when using a pointer; it doesn't happen when using the keyboard.
  2. Clicking outside a dialog closes them all.
Kapture.2025-01-09.at.15.12.38.mp4

In order to have two or more well-functioning modal dialogs opened at the same time, users need to nest them:

// first dialog
<Dialog.Root>
  <Dialog.Portal>
    <Dialog.Popup>

      // second dialog
      <Dialog.Root>...</Dialog.Root>
    </Dialog.Popup>
</Dialog.Root>

This is very limiting and users will expect to be able to open several dialogs at the same time without nesting them in the component tree:

// first dialog
<Dialog.Root>...</Dialog.Root>

// second dialog
<Dialog.Root>...</Dialog.Root>

Expected behavior

Like other libraries, users should be able to render several dialogs simultaneously without needing to nest them. Radix and other libraries support this: https://codesandbox.io/p/sandbox/radix-dialog-nested-without-nesting-jvvlrn?file=/src/App.tsx&workspaceId=ws_HE9JRzFW36Y2kpBoSwdvZ5

Reproducible example

Base UI version

v1.0.0-alpha.5

Additional context

Currently, data-nested, data-has-nested-dialogs, and --nested-dialogs are only functional when nesting dialogs in JSX. Having to nest dialogs in JSX is a big constraint for users (e.g., programmatically opened dialogs). While working on this issue, the team could think about allowing users to style dialogs using data-nested, data-has-nested-dialogs, and --nested-dialogs without actually nesting them in JSX.

@github-actions github-actions bot added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 10, 2025
@aarongarciah aarongarciah added the component: dialog This is the name of the generic UI component, not the React module! label Jan 10, 2025
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 10, 2025

Material UI / MUI Base supports this too: https://codesandbox.io/p/sandbox/unruffled-greider-hmdxgx?file=%2Fsrc%2FDemo.tsx%3A16%2C1.

I recall fixing a couple of bugs around this, it's likely going to prevent some teams from migrating. My guess is that people would hit this between 20–49% of real-life projects.

Could the root problem be that we need to bring back more test cases from Material UI: https://github.com/mui/material-ui/blob/60106b31d0b4e5e304fdb4d612cc62c1c21a20f6/packages/mui-material/src/Modal/Modal.test.js#L613 (not sure this reproduces this bug though) and Radix Primitives (didn't find it, but I don't know the codebase enough)?

@github-project-automation github-project-automation bot moved this to Backlog in Base UI Jan 13, 2025
@colmtuite colmtuite moved this from Backlog to In progress in Base UI Jan 13, 2025
@mj12albert mj12albert added new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dialog This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

4 participants