Multiple modals scrolling bug #1157
Comments
|
Yep, this makes sense. I think it is time to consider a stack for managing things like overlays and document level keybindings. Another example of this issue is pressing ESC. This will close all Dropdowns, Popups, Modals, and anything else that has a document level listener for ESC. Ideally, we'd have a stack of listeners and only the most recent would be invoked and removed. That way, only the top level component unmounts. This would allow opening/closing a Popup inside of a Modal without also closing the Modal. The stack could also be used here to keep the classes on the body until there were no more page dimmers in the stack. @jcarbo / @layershifter have you run into needs that would be well served by a stack or top-level Provider component for managing state such as listeners and multiple Portals? |
|
Will this be resolved to work by default in upcoming versions? |
|
If someone submits a PR it certainly will. If we run into this issue at @TechnologyAdvice then @davezuko or I may find time for it, though, we don't really use double modals nor have immediate plans to. I'd love to help support any community PR for this though. |
|
my temporary fix class ModalTrigger extends React.Component {
constructor(props) { ... }
componentWillUpdate() {
this.fixBody();
}
componentDidUpdate() {
this.fixBody();
}
fixBody() {
const anotherModal = document.getElementsByClassName('ui page modals').length;
if (anotherModal > 0) document.body.classList.add('scrolling', 'dimmable', 'dimmed');
}
render() {
return (
<Modal ... >
...
</Modal>
);
}
} |
|
If still not working well with @marcdibold fix, try bellow: class ModalTrigger extends React.Component {
constructor(props) { ... }
fixBody() {
const anotherModal = document.getElementsByClassName('ui page modals').length;
if (anotherModal > 0) document.body.classList.add('scrolling', 'dimmable', 'dimmed');
}
render() {
return (
<Modal onUnmount={this.fixBody} ... >
...
</Modal>
);
}
} |
|
This commit solves the issue with closing one of modal in presence of others. Unfortunately I don't know hot to make a PR out of 1 commit only (not the whole branch) (fix) When modal window unmounts it reverts portal's original class list, instead of resetting it |
|
Thanks for the workaround @rasdaniil. We've got an open PR to solve this correctly as well, by adding an event pool. We have this problem with many other things such as pressing ESC with an open Popup/Modal/Dropdown. Every component will close as they are all listening to that event. The open PR makes a stack of events and only fires the latest one. |
|
PR up in #2010 to fix this issue. |
|
Scrolling seems to not be working still. Checked with version 0.75.1. |
|
@rasdaniil This looks great and makes a lot more sense than what I hacked together. Open a PR since I'd rather a proper solution like yours merge than a hacky thing like mine. The idea was already brought up before and it makes sense. |
|
@RetroCraft I opened it some time ago but something went wrong :-) |
|
I am still experiencing this in 0.77.1 (pen here). Is this supposed to be fixed by now? (No offense, but I'm having a hard time telling from this issue tracker, there are a number of issues referencing each other, some of which are closed, some PRs merged, and I'm rather new to this. Also this bug is closed.) I guess we are waiting for the event pool PR? Or what is the status on this? Should one simply use the workaround by @rasdaniil right now and monkey-patch it locally? The event pool sounds like a larger task, but if I can do something to get the workaround into master for now, let me know and I'll try. Thanks heaps for publishing this lib by the way! |
|
@dfsp-spirit They do say 'contributions welcome' in the fine prints. This bug affects me as well. I have submitted #2394. Please test and tell me if that works for you. |
|
I am still experiencing this in 0.78.2 |
|
Was fixed in #2407, but not released yet. |
dimmable dimmedare deleted frombodywhen the second modal is closedSteps
Expected Result
The first modal should be scrollable
<body class="dimmable dimmed scrolling">Actual Result
First Modal is not longer scrollabel
<body class="scrolling">Version
tested in 0.64.0 & 0.63.5
Testcase
https://codepen.io/csxmedia/full/mRVgYX/
The text was updated successfully, but these errors were encountered: