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

[joy-ui] Forwarding ref to ModalOverflow breaks Select inside dialog #40790

Closed
zizhengtai opened this issue Jan 26, 2024 · 8 comments
Closed
Labels
component: modal This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy

Comments

@zizhengtai
Copy link

zizhengtai commented Jan 26, 2024

Steps to reproduce

Link to live example: https://codesandbox.io/p/sandbox/quirky-flower-j695xv?file=%2Fsrc%2FDemo.tsx%3A25%2C21

Steps:

  1. With <ModalOverflow ref={ref}> in the sample code, click on the <Select> inside the dialog, and see that the popup menu gets immediately closed. Look at the console and we can see the relevant blur event details.
  2. Remove the ref={ref} part, and the <Select> works again.

Current behavior

Forwarding ref to the <ModalOverflow> breaks <Select>s inside the dialog.

Expected behavior

This shouldn't happen.

Context

No response

Your environment

This happens to both Chrome and Firefox.

npx @mui/envinfo
  System:
    OS: macOS 14.2.1
  Binaries:
    Node: 20.11.0 - ~/.local/share/mise/installs/node/20/bin/node
    npm: 10.4.0 - ~/.local/share/mise/installs/node/20/bin/npm
    pnpm: 8.14.3 - ~/.local/share/mise/installs/node/20/bin/pnpm
  Browsers:
    Chrome: 120.0.6099.234
    Edge: Not Found
    Safari: 17.2.1
  npmPackages:
    @emotion/react: ^11.11.1 => 11.11.3
    @emotion/styled: ^11.11.0 => 11.11.0
    @mui/base:  5.0.0-beta.33
    @mui/core-downloads-tracker:  5.15.6
    @mui/joy: ^5.0.0-beta.8 => 5.0.0-beta.24
    @mui/private-theming:  5.15.6
    @mui/styled-engine:  5.15.6
    @mui/system:  5.15.6
    @mui/types:  7.2.13
    @mui/utils:  5.15.6
    @types/react: ^18.2.33 => 18.2.48
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
    typescript: ^5.2.2 => 5.3.3

Search keywords: joy-ui ModalOverflow Select

@zizhengtai zizhengtai added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 26, 2024
@zizhengtai
Copy link
Author

Workaround

Not forwarding the ref (i.e. not passing the ref variable to anything) seems to make everything work, but I don't know what the implication of intercepting the ref is. What's the proper way to handle refs when dealing with modals?

@zannager zannager added the package: joy-ui Specific to @mui/joy label Jan 26, 2024
@ZeeshanTamboli
Copy link
Member

Why do you need to pass ref to ModalOverflow in the first place?

@ZeeshanTamboli ZeeshanTamboli added status: waiting for author Issue with insufficient information component: modal This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Feb 6, 2024
@zizhengtai
Copy link
Author

Why do you need to pass ref to ModalOverflow in the first place?

The custom component that I pass to Modal as the child needs to hold a ref (otherwise Modal throws an error), so naturally I assumed that passing the ref down is important. Is that not the case?

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Feb 7, 2024
@ZeeshanTamboli
Copy link
Member

I assumed that passing the ref down is important. Is that not the case?

It's not required. It's not even mentioned in the docs. Not sure where you got this idea from. After removing the ref it works fine and does not throw error on Modal: https://codesandbox.io/p/sandbox/gifted-https-cyp8xj.

@ZeeshanTamboli ZeeshanTamboli removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 7, 2024
@zizhengtai
Copy link
Author

@ZeeshanTamboli good to know it's not required, but why does <Modal> require its child can hold a ref in the first place?

@ZeeshanTamboli
Copy link
Member

why does require its child can hold a ref in the first place

I'm not quite following. The ref is included in the children of the Modal to enable access to those children components' DOM.

@zizhengtai
Copy link
Author

zizhengtai commented Feb 7, 2024

@ZeeshanTamboli apologies if this is a noob question. As a concrete example, the following snippet

function MyDialog() {
  return ...
}

...

return <Modal><MyDialog /></Modal>

results in a runtime error that MyDialog can't hold a ref and that I should wrap it in a forwardRef. Now that MyDialog looks like this:

const MyDialog = forwardRef(function MyDialog(_, ref) {
  return ...
})

I thought I should pass that ref down to MyDialog's children in order for the parent Modal to function correctly, because otherwise why would it ask me to use forwardRef in the first place?

@ZeeshanTamboli
Copy link
Member

@zizhengtai Can you share a CodeSandbox throwing that runtime error?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: modal This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy
Projects
None yet
Development

No branches or pull requests

4 participants