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

[Bug]: Dialog doesn't replace backdrop, just overlaps it #33734

Open
philkuo opened this issue Jan 27, 2025 · 7 comments
Open

[Bug]: Dialog doesn't replace backdrop, just overlaps it #33734

philkuo opened this issue Jan 27, 2025 · 7 comments

Comments

@philkuo
Copy link

philkuo commented Jan 27, 2025

Component

Dialog

Package version

9.5.12

React version

17.0.45

Environment

GitHub Codespace

  System:
    OS: Linux 6.5 Ubuntu 22.04.5 LTS 22.04.5 LTS (Jammy Jellyfish)
    CPU: (32) x64 AMD EPYC 7763 64-Core Processor
    Memory: 114.90 GB / 125.78 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Browsers:
    Chrome: 131.0.6778.85

Current Behavior

<DialogSurface>'s backdrop attribute is an element that gets appended as a child to the existing default dialog backdrop element.

Expected Behavior

<DialogSurface>'s backdrop element should REPLACE the default dialog backdrop element, not be a child of it.

The documentation says this:

This slot expects a <div> element which will replace the default backdrop.
Source: https://react.fluentui.dev/?path=/docs/components-dialog--docs

Reproduction

https://stackblitz.com/edit/p154gpbz-9uw2txdb?file=src%2Fexample.tsx

Steps to reproduce

The below code uses the backdrop attribute. Since it's unstyled and should be replacing the default backdrop, theoretically the background should not be dimmed at all when the dialog is open, but it is. You can see the foobar div as a child of the default backdrop instead of replacing it.

    <Dialog>
      <DialogTrigger>
        <Button>Open dialog</Button>
      </DialogTrigger>
      <DialogSurface backdrop={<div>foobar</div>}>
        <DialogBody>
            Lorem ipsum
          <DialogActions>
            <DialogTrigger>
              <Button>Close</Button>
            </DialogTrigger>
          </DialogActions>
        </DialogBody>
      </DialogSurface>
    </Dialog>

### Are you reporting an Accessibility issue?

no

### Suggested severity

High - No workaround

### Products/sites affected

SharePoint

### Are you willing to submit a PR to fix?

no

### Validations

- [x] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
- [x] The provided reproduction is a minimal reproducible example of the bug.
@dmytrokirpa
Copy link
Contributor

@philkuo, Thanks for pointing this out!

Basically, this won't replace the slot element, but instead use it as children for a slot:

<DialogSurface backdrop={<div>foobar</div>}>

And this will completely replace the slot element:

<DialogSurface backdrop={{ children: (_, props) => <div>foobar</div> }}>

Please check out the slots dev docs for more usage examples:

https://react.fluentui.dev/?path=/docs/concepts-developer-customizing-components-with-slots--docs#usage-examples

especially

https://react.fluentui.dev/?path=/docs/concepts-developer-customizing-components-with-slots--docs#replacing-the-entire-slot

The prop description seems a bit confusing, so we should probably look at making it clearer.

@philkuo
Copy link
Author

philkuo commented Jan 29, 2025

I get this error:

Uncaught Error: @fluentui/react-motion: Invalid child element. Motion factories require a single child element to be passed. That element element should support ref forwarding i.e. it should be either an intrinsic element (e.g. div) or a component that uses React.forwardRef().

Using this code:

            <DialogSurface
              backdrop={
                // <div aria-hidden className={cssStyles.overlay} />
                { children: (unused, props) => <div aria-hidden className={cssStyles.overlay} /> }
              }
            >

I tried this as well but it had the same error:

{ children: (unused, p) => <div aria-hidden ref={p.ref} className={cssStyles.overlay} /> }

@dmytrokirpa
Copy link
Contributor

Hey @philkuo, please check out your updated example here: https://stackblitz.com/edit/p154gpbz-2nikxz6e?file=src%2Fexample.tsx and let us know if you're still having any issues

@philkuo
Copy link
Author

philkuo commented Jan 29, 2025

I tested the permutations of this. It seems there's no way to have a custom backdrop AND have the backdrop animated then (either via createPresenceComponent() or leaving it as the default animation)?

@philkuo
Copy link
Author

philkuo commented Jan 29, 2025

I also tried this with the props but it also did not work:
{ children: (unused, p) => <div aria-hidden {...p} className={cssStyles.overlay} /> }

@dmytrokirpa
Copy link
Contributor

dmytrokirpa commented Jan 29, 2025

I also tried this with the props but it also did not work: { children: (unused, p) => <div aria-hidden {...p} className={cssStyles.overlay} /> }

The original issue was "Dialog doesn't replace backdrop, just overlaps it" and here's a working example https://stackblitz.com/edit/p154gpbz-8kkpf78t?file=src%2Fexample.tsx,src%2FApp.tsx. Is this not what you need? Are you looking for a way to customize the backdrop motion/animation?

@philkuo
Copy link
Author

philkuo commented Jan 29, 2025

I do not need a custom animation, the default is fine. I do need a custom backdrop, but it must have an animation (default is fine). It seems it is impossible to have just the custom backdrop (without the default backdrop) AND an animation.
I did not state the requirement to have an animation up front since I did not know that would be a blocking issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants