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

[Portal] Don't use document.body as a default container #21626

Closed
TomPradat opened this issue Jun 30, 2020 · 1 comment
Closed

[Portal] Don't use document.body as a default container #21626

TomPradat opened this issue Jun 30, 2020 · 1 comment
Labels
component: Portal The React component.

Comments

@TomPradat
Copy link
Contributor

https://github.com/mui-org/material-ui/blob/c9e218142e98f4bef2ec31852bdcc0a3290f3742/packages/material-ui/src/Portal/Portal.js#L27

I'm not 100% sure that it's a bad idea, but according to react documentation on ReactDOM.render :

ReactDOM.render() controls the contents of the container node you pass in. Any existing DOM elements inside are replaced when first called. Later calls use React’s DOM diffing algorithm for efficient updates.

And this stackorverflow post

Are you sure it's safe ? Wouldn't it be better/safer to just create a div in the body instead or throw an error ?
@oliviertassinari oliviertassinari added the component: Portal The React component. label Jun 30, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 30, 2020 •
edited

@TomPradat Closing as no actual error related to it has been reported in 3 years since we use this logic. It's safe until proven otherwise.

jesec added a commit to jesec/flood that referenced this issue Aug 20, 2020
@jesec
Tooltip: simplify Portal creation
6796391 
It is unneccessary and unreliable to create an extra div to hold Portal. The
presumed benefits[1] of an extra div are never proven and developers never
find an *ACTUAL* case where putting Portal to the body poses a problem[2].

In our case, creation of this extra div (or the logics to do so) is the
*ACTUAL* problem. Somehow the "ref" inside this div doesn't work reliably.
As a result, getIdealLocation and handleMouseEnter fail.

[1]: https://stackoverflow.com/questions/49504546/is-it-safe-to-use-reactdom-createportal-with-document-body
[2]: mui/material-ui#21626

Bug: #15
@PiotrKozlowski PiotrKozlowski mentioned this issue Nov 25, 2020
Merged
@AVor0n AVor0n mentioned this issue Dec 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: Portal The React component.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@oliviertassinari @TomPradat