Semantic-ui-react: Popup's do not work with modal's when dimmers are enabled.

Created on 28 Jun 2017  路  19Comments  路  Source: Semantic-Org/Semantic-UI-React

This is a follow up to this issue:
https://github.com/Semantic-Org/Semantic-UI-React/issues/1545

The submitter was forwarded to /Semantic-UI
I followed up in this post:
https://github.com/Semantic-Org/Semantic-UI/issues/5440

It turns out that the issue has to do with the order the the popups/modals are created in.
/Semantic-UI responded that pre-existing modals should be used for this.
As far as i know, there's no way to achieve that in the current react variant of the UI.

Thank you :)

Steps

  • Create a modal with a dimmer setting that's not false.
  • Place a pop up inside the modal

Expected Result

Hovering over trigger displays the popup on top of the modal

Actual Result

Popup displays behind the modal, even behind the dimmer actually!

Version

0.70.0

Testcase

https://codepen.io/eGust/pen/zZXomK

help wanted needs engineering stale

Most helpful comment

Ugh - just found a link to this issue going through my code. Anyone up for taking a look at this? It would be pretty awesome 馃槑 . Posting mainly to de-stale this issue ... I'd take it on but I think the css is a little out of my depth.

All 19 comments

@Vino4 thanks for creating this issue. I've had the same problem. We're using rc-time-picker and have just added a Modal around that component.

The pop-up on the time picker now only shows if the dimmer on the modal is set to false.

Update for anyone following: I've figured out this bug only happens for us when the dimmer is set to "blurring". If it's just left as the default dimmer it works as expected.

@Vino4 @katpas There is a way that it could work now with blurring dimmer?

Oh boy, this is a tricky issue. In short, the CSS for a blurring dimmer will blur anything _outside_ of the ui page modals dimmer transition visible active but the Portal renders first the Dimmer, then the Popup both to the body as _siblings_.

The Popup is, therefore, _outside_ of the dimmer and is blurred by the dimmable dimmed blurring classes on the `body.

In this case, the Popup needs to be rendered into the Modal, with the Modal as its mountNode. However, there is a catch 22 here. The Modal creates the Popup.

In order to do this currently, you'd have to have some ugly logic. First, the Modal needs to be rendered, with some unique id so you can find the DOM node. Then, onMount, you need to query the DOM for the Modal's DOM node so you can set the Popup's mountNode to the Modal. Case in point: https://codepen.io/levithomason/pen/WZJpLN

However, this also is insufficient as the Popup currently doesn't calculate its position based on the mountNode, but assumes the body to be the mountNode.

Workaround

Use a <Modal dimmer="inverted" /> as it is closer to blurring than the default and works. The default Dimmer also works.

Fix

The best path here is likely to update the CSS for blurring dimmers. Right now, it blurs anything that is not a dimmer:

.blurring.dimmed.dimmable > :not(.dimmer) {
    filter: blur(5px) grayscale(0.7);
}

Simply allowing a className like not blurred to escape this rule would allow us to ensure Popup portals are not blurred.

I would suggest posting a bug on SUI core and linking this suggestion to it.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.

Ugh - just found a link to this issue going through my code. Anyone up for taking a look at this? It would be pretty awesome 馃槑 . Posting mainly to de-stale this issue ... I'd take it on but I think the css is a little out of my depth.

I found a way to make it work. Set z-index of the popup the largest.

@pdfowler @aryalprakash per the comment above, either of you might want to try doing this:

I would suggest posting a bug on SUI core and linking this suggestion to it.

Because of the chicken or the egg situation with how the popup is rendered, this is something that will have to be fixed in CSS.

According to SUI 761, you can set the popup to display inline, rather than attached to the body tag. However, inline is not a valid property on <Popup ...> element, resulting in warning.js?6327:33 Warning: Unknown propinlineon <div> tag ...

I swore I had seen a way to attach the popup to an element, rather than to the body, but maybe that was on another component. Anyone have thoughts on this path?

Also, @aryalprakash, I'm not sure how you got z-index to work since as @levithomason mentioned, even if the popup is at the "top" of the z-index stack, it will still be blurred per the css implementation of dimmer. Inspecting the elements, my modal's dimmer has a default z-index: 1005, while the popup has a default z-index: 1900, so a manual override _should_ not make any difference. (But clearly, YMMV)

Inline popups are something I've been wanting as well because if you have a pop-up in something like a scrolling sidebar, the pop-up doesn't stick to the element

Looking at the history of this, it looks like inlining was an issue from the original PR

inline (popup as a sibling of trigger, will be implemented later)

... with the discussion continuing around a possible mountNode param, along with the difficulties of implementation, @debrice noted his concerns:

My concern is offering a mountNode props might not be a feature we want to keep as I would rather have an boolean inline option in the long run. The inline option would make the popup a sibling element of the the trigger and require a bit more kung-fu for positioning but would also be more intuitive and closer to the spirit of SUI. I'm not sure how long it would take me to build a clean inline feature so I would rather deliver the popup without it initially.

While other discussions address injecting the Portal adjacent to the trigger. @debrice asked:

can you think of an easy way to have the portal injected as a sibling to the trigger? That's might be the last thing that prevents me from implementing the inline feature

Question with those more familiar with the internals, engineering wise, is this an issue that the Portal component can now support? I assume it is not as simple as adding the {inline: true} per SUI 761, as it involves JS implementation in the SUI-React package. Is this correct?

edit: It appears that Portal does support the mountNode property, could this be as simple as passing the trigger node to mountNode of the Portal, if inline === true in the Popup props?

edit2: After reviewing @levithomason comments above, sounds like there's some additional Fu involved. Hoping to find some time this weekend to take a look, but any further guidance would be appreciated 馃槑

Hi,

Anyone found a solution yet? I have been struggling with this issue as well..

I have the same issue... Could it be fixed, please?

There has been no activity in this thread for 180 days. While we care about every issue and we鈥檇 love to see this fixed, the core team鈥檚 time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 180 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

Anyone who's still on this, a possible workaround could be to set filter: none css property on the popup.

Not sure when this was resolved, but it is resolved in the latest release 0.85.0:

http://g.recordit.co/2QUytUKWGn.gif

Spoke a bit too soon. Default dimmers do work, as shown above.

inverted dimmers work:

http://g.recordit.co/25SBzmlrSa.gif

However, blurring dimmers still have an issue:

http://g.recordit.co/wCZMmroFnU.gif

My original diagnosis and proposed solution still seems relevant: https://github.com/Semantic-Org/Semantic-UI-React/issues/1803#issuecomment-335371082

We fixed this for Fomantic-UI by adding :not(.popup) to the existing selector

.blurring.dimmed.dimmable > :not(.dimmer):not(.popup) {
    filter: blur(5px) grayscale(0.7);
}

See https://jsfiddle.net/s48j9b5v

More explanation in the related PR https://github.com/fomantic/Fomantic-UI/pull/616

This worked for me:

.blurring.dimmed.dimmable > :not(.dimmer).popup {
  filter: none;
}

There has been no activity in this thread for 180 days. While we care about every issue and we鈥檇 love to see this fixed, the core team鈥檚 time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 180 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

Was this page helpful?
0 / 5 - 0 ratings