diff --git a/packages/material-ui/src/Modal/ModalManager.js b/packages/material-ui/src/Modal/ModalManager.js index 9d0e9982b493de..2fdad7524449af 100644 --- a/packages/material-ui/src/Modal/ModalManager.js +++ b/packages/material-ui/src/Modal/ModalManager.js @@ -59,27 +59,8 @@ function handleContainer(containerInfo, props) { let fixedNodes; if (!props.disableScrollLock) { - const overflowing = isOverflowing(container); - - // Improve Gatsby support - // https://css-tricks.com/snippets/css/force-vertical-scrollbar/ - const parent = container.parentElement; - const scrollContainer = - parent.nodeName === 'HTML' && window.getComputedStyle(parent)['overflow-y'] === 'scroll' - ? parent - : container; - - restoreStyle.push({ - value: scrollContainer.style.overflow, - key: 'overflow', - el: scrollContainer, - }); - - // Block the scroll even if no scrollbar is visible to account for mobile keyboard - // screensize shrink. - scrollContainer.style.overflow = 'hidden'; - - if (overflowing) { + if (isOverflowing(container)) { + // Compute the size before applying overflow hidden to avoid any scroll jumps. const scrollbarSize = getScrollbarSize(); restoreStyle.push({ @@ -97,6 +78,23 @@ function handleContainer(containerInfo, props) { node.style.paddingRight = `${getPaddingRight(node) + scrollbarSize}px`; }); } + + // Improve Gatsby support + // https://css-tricks.com/snippets/css/force-vertical-scrollbar/ + const parent = container.parentElement; + const scrollContainer = + parent.nodeName === 'HTML' && window.getComputedStyle(parent)['overflow-y'] === 'scroll' + ? parent + : container; + + // Block the scroll even if no scrollbar is visible to account for mobile keyboard + // screensize shrink. + restoreStyle.push({ + value: scrollContainer.style.overflow, + key: 'overflow', + el: scrollContainer, + }); + scrollContainer.style.overflow = 'hidden'; } const restore = () => { diff --git a/packages/material-ui/src/Modal/ModalManager.test.js b/packages/material-ui/src/Modal/ModalManager.test.js index 9d3ef4915fde9f..cc5c994b413a85 100644 --- a/packages/material-ui/src/Modal/ModalManager.test.js +++ b/packages/material-ui/src/Modal/ModalManager.test.js @@ -135,6 +135,29 @@ describe('ModalManager', () => { assert.strictEqual(fixedNode.style.paddingRight, '14px'); }); + it('should disable the scroll even when not overflowing', () => { + // simulate non-overflowing container + const container2 = document.createElement('div'); + Object.defineProperty(container2, 'scrollHeight', { + value: 100, + writable: false, + }); + Object.defineProperty(container2, 'clientHeight', { + value: 100, + writable: false, + }); + document.body.appendChild(container2); + + const modal = {}; + modalManager.add(modal, container2); + modalManager.mount(modal, {}); + assert.strictEqual(container2.style.overflow, 'hidden'); + modalManager.remove(modal); + assert.strictEqual(container2.style.overflow, ''); + + document.body.removeChild(container2); + }); + it('should restore styles correctly if none existed before', () => { const modal = {}; modalManager.add(modal, container1);