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

Popover Elements (Tooltip, Popover, Overlay) do not open with Portals #3248

Closed
mulholo opened this issue Dec 17, 2018 · 21 comments
Closed

Popover Elements (Tooltip, Popover, Overlay) do not open with Portals #3248

mulholo opened this issue Dec 17, 2018 · 21 comments

Comments

@mulholo
Copy link

mulholo commented Dec 17, 2018

Environment

  • Package version(s): 3.10
  • Browser and OS versions: Chrome, Firefox and Safari (latest). MacOS 10.14.1 (latest)

Steps to reproduce

  1. Use <Popover /> anywhere with usePortal={true} (default behaviour)
  2. Click on the popover target

Actual behavior

  • No popover content appears when clicked
    -bp3-popover-open class still appears on the target
  • No portal div appears at the bottom of the page
  • Setting the usePortal prop to true gets rid of the bug.

Expected behaviour

  • Popover opens within a portal div and is visible.

Possible solution

  • I supect this is not a CSS issue as the problem seems to be that the portal div is not being added at all to the bottom of the page.

Other Information

@mulholo mulholo changed the title Popover Elements (Tooltip, Popover, Overlay) Popover Elements (Tooltip, Popover, Overlay) do not open with Portals Dec 18, 2018
@mulholo
Copy link
Author

mulholo commented Dec 19, 2018

Still not resolved, but we have found that by setting portalContainer={document.body} (the default) portals render correctly.

@giladgray
Copy link
Contributor

@mulholio i have never heard of this issue. my gut says it's something weird with your environment. can you please attempt to create a repro on codesandbox? https://codesandbox.io/s/rypm429574

@mulholo
Copy link
Author

mulholo commented Dec 21, 2018

Thanks for the link. I have tried copying problematic code from two different parts of our app and haven't succeeded in reproducing the bug in Codesandbox. Given this, I suspect your analysis is correct and our environment is doing something strange.

Closing the issue, but was wondering if you had any ideas on what the cause might be?

@mulholo mulholo closed this as completed Dec 21, 2018
@nikko242
Copy link

I have the exact same behaviour as you @mulholio

Have you figured out what was wrong in your environment, and how to fix it?

Regards
Niklas

@mulholo
Copy link
Author

mulholo commented Feb 14, 2019

@nikko242 no resolution yet. I'm just relying upon the usePortal prop for now as a work around. Give us a shout if you find a better fix!

@nikko242
Copy link

Ok, thanks.

No, we also use usePortal together with portalContainer to make it work.

I have had a look at the code, and it seems strange to me that it could work on a different environment, it seems that portalContainer never gets set to "document.body" as default as stated in the documentation.

@mulholo
Copy link
Author

mulholo commented Feb 20, 2019

@nikko242 Any luck on this? Was hoping to resolve the issue just with usePortal={false} but the Overlay component is used behind the scenes on other Blueprint Components (e.g. Table Cell Context Menus) so we're running into still.

We've deleted all the CSS so it doesn't appear to be anything related to that.

I can find the underlying Portal and children in React dev-tools but mousing over just shows 0px x 0px at the top left and doesn't actually render the content on the page.

@mulholo
Copy link
Author

mulholo commented Feb 21, 2019

Further investigations:

  • From inspecting Blueprint's source code and stepping through in dev tools, I can see that in the <Portal /> render method the portal is only created if this condition is false:
    if (cannotCreatePortal || typeof document === "undefined" || !this.state.hasMounted) {
  • In my app, the is true as state.hasMounted is false. This is because props.container is null (see componendDidMount() for this)

Still trying to work out why the container prop is missing.

@nikko242
Copy link

nikko242 commented Feb 22, 2019

We work around this, by manually settings portalContainer when Portal=true, it works .. but the code gets abit bloated:

<Dialog
                   onClose={this.props.OnClose}
                    title="Ändra skipass"
            isOpen={true}
            usePortal={true}
            portalContainer={document.body}>...               
</Dialog>

@nikko242
Copy link

I have taken a look in the blueprinjs source code, and I think It would be quite easy to fix:

In "packages/core/src/components/overlay/overlay.tsx"
line: 231
<Portal className={this.props.portalClassName} container={this.props.portalContainer}>
Change to:
<Portal className={this.props.portalClassName} container={this.props.portalContainer != null?this.props.portalContainer:document.body}>

@mulholo
Copy link
Author

mulholo commented Feb 22, 2019

Would the Blueprint team be open to adding this to the Overlay component?

Before:

<Portal className={this.props.portalClassName} container={this.props.portalContainer}>

After:

<Portal
  className={this.props.portalClassName}
  container={this.props.portalContainer ? this.props.portalContainer : document.body}
>

At the very least, I feel like there should be some kind of error thrown if you try and create a portal with no container element referenced. As suggested by @nikko242 , document.body seems like a sensible default to create a portal on if none is explictly passed?

Happy to make a PR.

@mulholo mulholo reopened this Feb 22, 2019
@adidahiya
Copy link
Contributor

@mulholio that default gets set further down the component stack:

container: typeof document !== "undefined" ? document.body : null,
. so I don't think your suggested change would fix this.

but I will admit it's confusing for Overlay's docs to list @default document.body for its portalContainer prop when it doesn't actually do any default handling itself (only Portal does that).

@chiubaka
Copy link

chiubaka commented Apr 4, 2019

I'm also encountering this issue. Setting document.body explicitly seems to resolve. If it helps, things break for me starting with 3.8.0. They work in 3.7.0. I'm therefore fairly certain the problem was introduced by #3045.

@adidahiya
Copy link
Contributor

If you are still seeing this issue can you create a repro of it in a code sandbox (see link in the README or issue template) or a small repo?

@chiubaka
Copy link

This is still an issue for me. Trying to see what I can do to simplify my own configuration and pinpoint the issue. Thus far, my best guess is that it somehow has something to do with Rails + Webpacker + Blueprint... I'm unable to get even a simple example (page literally just displays a Popover Button, all other JS/TS/CSS/SCSS code has been removed, and all unnecessary packages have been removed) to work with this pipeline using the latest versions of everything.

Setting document.body explicitly still works, but I'm out of luck without that (and not all of the popover components expose the portalContainer prop, particularly the DateInput and the like, I think).

Any chance anyone else with this issue is using a similar setup? May try to see if I can create a fresh small repo with this, if for no other reason than to test my own sanity...

@chiubaka
Copy link

chiubaka commented Apr 29, 2019

Here's the smallest possible Rails + Webpack + Blueprint repo I could create. To my chagrin, still seeing the issue here. I've done the absolute minimum of additional configuration outside of the defaults. Would love to hear that the Rails defaults just misconfigure Babel or Webpack somehow... or that I did something super dumb and unexpected.

Since Code Sandbox doesn't support this sort of environment, I can't replicate this there. I'm not able to reproduce my issue with Code Sandbox's pre-built envs.

@chshanovskiy
Copy link

Hi, i've encountered same issue with my own setup. Usually, bundle injected in end of body, and div#root is present in layout.
But in my setup, I've injected bundle to head, and wrap my React.render with DOMContentLoaded.
It's hard to reproduce on codesandbox, because of custom webpack-html-plugin options

When I reverted setup to "defaults", problem is gone.

Hope this would be helpful

@mulholo
Copy link
Author

mulholo commented May 2, 2019

@chiubaka we were getting this issue on a Rails app too so sounds like you might be onto something. I've left the project recently but @strickland84 may have some relevant details.

@chiubaka
Copy link

chiubaka commented May 2, 2019

Ah... well, that's interesting. So actually including <%= javascript_pack_tag "application" %> in the HTML head causes document.body to be initially null when all the scripts run. I think this is causing Blueprint some headache... likely there's some initialization code for the Portal that doesn't like this.

Moving <%= javascript_pack_tag "application" %> to the bottom of the body seems to solve the issue.

@adidahiya
Copy link
Contributor

Generally it's best practice to inject application scripts at the bottom of body anyway, so as to not block HTML rendering.

@tgalopin
Copy link

tgalopin commented Nov 7, 2021

I'm encountering this issue as well, but only when I'm using https://github.com/hotwired/turbo. Because of the way Turbo works, I put the script tags at the end of the head tag with the defer attribute. Given that people experiencing this issue seems to come from Rails, where Turbo originates from, it could make sense that the common denominator is the usage of Turbo.

By investigating a bit more, I think I have more details:

  • when the issue occurs, it's because the original React element (from my app) doesn't seem to have a usable size on the page (as you can see in the following image, the React Dev Tools display NaN as a position/size for the element)
  • the issue does not happen at the first load (ie when the page is loaded normally) but happens afterwards, when Turbo is used to load the page and replace its content
  • the issue is resolved, even with Turbo, when using portalContainer={document.body}
  • I'm using the 4.0-beta8

image

I'm not sure what's the source cause but having portalContainer={document.body} fixing the problem is nice :) .

Daru13 added a commit to exsitu-projects/lorgnette that referenced this issue Oct 26, 2023
There seems to be an issue with the opening of portals in certain environements. The problem only occurs in the production build; never in the development build. It may be related to the following (open) issue: palantir/blueprint#3248.
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

7 participants