Is there any documentation regarding this functionality out there? I saw a mention of it in the modal api reference but can't find when/how to use it.
@fltonii We have no documentation about it as we are speaking. I'm not sure it's something that is worth mentioning in the API page of the modal. The only "valid" use case I have seen so far is around people bundling two versions of Material-UI, realizing that their modal logic isn't valid (as it can only have one top model normally). This issue can be "solved" with a global variable.
@mui-org/core-contributors What do you think of adding @ignore in the Modal prop-types to hide the existence of this entity to our users?
@oliviertassinari we are using this on my project to disable the overflow style change to body. It was causing a huge performance issue when opening and closing modals. Is there another way to do this?
@jeffshaver Do you know why it was causing a performance issue?
We could add a property for this.
@olivertassinari I'm not 100% sure. But after figuring out the style change on body was causing it, I thought it was probably that changing styles on body cause a reflow and repaint of everything.
I also need to be able to disableScrollLock. Sounds good with a property for this.
@Sojborg If you want to give it a shot, it shouldn't be harder than:
--- a/packages/material-ui/src/Modal/Modal.js
+++ b/packages/material-ui/src/Modal/Modal.js
@@ -213,6 +213,7 @@ class Modal extends React.Component {
disableEscapeKeyDown,
disablePortal,
disableRestoreFocus,
+ disableScrollLock,
hideBackdrop,
innerRef,
keepMounted,
@@ -354,6 +355,10 @@ Modal.propTypes = {
*/
disableRestoreFocus: PropTypes.bool,
/**
+ * Disable the scroll lock behavior.
+ */
+ disableScrollLock: PropTypes.bool,
+ /**
* If `true`, the backdrop is not rendered.
*/
hideBackdrop: PropTypes.bool,
@@ -412,6 +417,7 @@ Modal.defaultProps = {
disableEscapeKeyDown: false,
disablePortal: false,
disableRestoreFocus: false,
+ disableScrollLock: false,
hideBackdrop: false,
keepMounted: false,
// Modals don't open on the server so this won't conflict with concurrent requests.
diff --git a/packages/material-ui/src/Modal/ModalManager.js b/packages/material-ui/src/Modal/ModalManager.js
index 0a381abb9..f6334de0e 100644
--- a/packages/material-ui/src/Modal/ModalManager.js
+++ b/packages/material-ui/src/Modal/ModalManager.js
@@ -75,10 +75,9 @@ function removeContainerStyle(data) {
*/
class ModalManager {
constructor(options = {}) {
- const { hideSiblingNodes = true, handleContainerOverflow = true } = options;
+ const { hideSiblingNodes = true } = options;
this.hideSiblingNodes = hideSiblingNodes;
- this.handleContainerOverflow = handleContainerOverflow;
// this.modals[modalIdx] = modal
this.modals = [];
@@ -130,7 +129,7 @@ class ModalManager {
const containerIdx = findIndexOf(this.data, item => item.modals.indexOf(modal) !== -1);
const data = this.data[containerIdx];
- if (!data.style && this.handleContainerOverflow) {
+ if (!data.style && !modal.props.disableScrollLock) {
setContainerStyle(data);
}
}
@@ -150,7 +149,7 @@ class ModalManager {
// If that was the last modal in a container, clean up the container.
if (data.modals.length === 0) {
- if (this.handleContainerOverflow) {
+ if (!data.style && !modal.props.disableScrollLock) {
removeContainerStyle(data);
}
Most helpful comment
@fltonii We have no documentation about it as we are speaking. I'm not sure it's something that is worth mentioning in the API page of the modal. The only "valid" use case I have seen so far is around people bundling two versions of Material-UI, realizing that their modal logic isn't valid (as it can only have one top model normally). This issue can be "solved" with a global variable.
@mui-org/core-contributors What do you think of adding
@ignorein the Modal prop-types to hide the existence of this entity to our users?