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

Respect Node.ownerDocument of anchorEl when creating portal for Menu #9207

Closed
1 task done
ianschmitz opened this issue Nov 18, 2017 · 11 comments
Closed
1 task done
Assignees
Labels
bug 🐛 Something doesn't work component: Portal The React component.

Comments

@ianschmitz
Copy link
Contributor

  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

When opening a Menu and creating a react portal, the portal that's created should reference the Node.ownerDocument of the anchorEl or something similar, to support rendering Material-UI components in a new window (such as through a library like react-popout)

Current Behavior

The portal that is created always uses the global document object to append a portal div to. See: https://github.com/callemall/material-ui/blob/08fae8c2fcd741c01cabbee38305d5ef109acfd5/src/internal/Portal.js#L45-L55

Steps to Reproduce (for bugs)

A simple example can be seen here: https://codesandbox.io/s/v09qklqkj7. You can ignore the lack of styling and what not in the popout window, just trying to show the portal issue.

Your Environment

Tech Version
Material-UI 1.0.0-beta.21
React 16
browser All
@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 18, 2017

Wow, I wasn't aware of such thing, thanks for sharing! I find it such an edge case that I might never work on fixing it. Have you found a simple work-around?
Maybe something like: https://github.com/react-bootstrap/react-overlays/blob/master/src/Portal.js#L48-L50

@ianschmitz
Copy link
Contributor Author

Ahh darn i didn't see the link you provided before I started prototyping. Their solution looks very interesting. I'll see if that works well for us and update the PR accordingly.

@corytheboyd
Copy link

I also ran into this same problem recently. Our project is a Chrome extension, and we render our UI inside of an iframe on the page to avoid inheriting that page's styles, which as you could imagine, breaks the styles added by Material UI in wonderful amazing ways.

This introduced two problems:

  1. JSS injects stylesheets into the top-level document
  2. Material UI Portal renders into the top-level body

We were able to first the first issue by configuring JSS's insertion point to use the iframe's document, but the second issue is exactly what you are addressing with this PR.

I agree that rendering Material UI components into an iframe is an edge-case, but I don't think it should be completely discounted either, as it provides a lot of power when building applications more complicated than a single page application.

@ianschmitz
Copy link
Contributor Author

I think the PR (#9236) is in a pretty decent state now.

@corytheboyd do you have a sense if we took the same approach with injecting JSS (respecting Node.ownerDocument), that it might solve your first problem?

@corytheboyd
Copy link

@ianschmitz Unfortunately not, the root cause of our JSS issue is not being able to configure the JSS instance that Material UI uses internally. It would be wonderful if you could specify a document through the theme provider (or some other similar provider component) and just have that be the document that the rest of the code references.

@oliviertassinari oliviertassinari added component: Portal The React component. bug 🐛 Something doesn't work labels Dec 7, 2017
@oliviertassinari oliviertassinari added this to the v1.0.0-prerelease milestone Dec 7, 2017
@gregnb
Copy link
Contributor

gregnb commented Dec 8, 2017

I'm currently suffering from an issue somewhat close to what you guys have. So picture a grid system with: left rail, content, and right rail. My left rail container is position: fixed. Inside my left rail I have a tool bar, now each of these toolbar icons use the MUI Popover component which appends to the body. The placement of the popover becomes an issue as the user starts to scroll down the page because what happens is:

  1. The scroll shoots right back to the top of the page
  2. Then the popover shows up

What I really need is the ability to tell Popover that I would like to inject into a different target other than mui-portal. Or maybe even have a target for mui-portal other than body. Then, I believe, when a user would click on my toolbar the Popover would be positioned relative correctly if I could set a target div of my left rail instead of body.

An example of I'm thinking of is the way Popover works in bootstraphttp://bootstrapdocs.com/v3.0.3/docs/javascript/#popovers where you can specify a 'container' target.

I was searching for answers in the issues and stumbled upon this. @oliviertassinari if you have another way I could solve this I would love to hear. Thanks!

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 20, 2018

I'm looking into making Material-UI works in an iframe. Has anyone successfully achieve it?

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 20, 2018

Nevermind, I have found a way :).

@ianschmitz
Copy link
Contributor Author

@oliviertassinari the tricky part is getting JSS to play nicely. Let me know if you have questions!

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 20, 2018

I'm on it #13314. JSS insertion point do the job. Now, I need to set the right container for the portal.

@oliviertassinari
Copy link
Member

Problem solved too :)

capture d ecran 2018-10-20 a 16 25 39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: Portal The React component.
Projects
None yet
Development

No branches or pull requests

4 participants