From 033f9014b9951112da610435e0360f5ce463232b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Pradel?= Date: Sun, 15 Nov 2020 14:59:43 +0100 Subject: [PATCH] feat: use body-scroll-lock instead of no-scroll (#455) --- .../__tests__/index.test.tsx | 28 +++++++++---------- .../cypress/integration/modal.spec.ts | 12 ++++---- react-responsive-modal/package.json | 9 +++--- react-responsive-modal/src/index.tsx | 24 ++++++---------- react-responsive-modal/src/modalManager.ts | 27 ++++++------------ react-responsive-modal/src/useScrollLock.ts | 24 ++++++++++++++++ react-responsive-modal/src/utils.ts | 10 ------- yarn.lock | 24 ++++++++++------ 8 files changed, 82 insertions(+), 76 deletions(-) create mode 100644 react-responsive-modal/src/useScrollLock.ts diff --git a/react-responsive-modal/__tests__/index.test.tsx b/react-responsive-modal/__tests__/index.test.tsx index a989dd10..a2021677 100644 --- a/react-responsive-modal/__tests__/index.test.tsx +++ b/react-responsive-modal/__tests__/index.test.tsx @@ -105,7 +105,7 @@ describe('modal', () => {
modal content
); - expect(document.documentElement.style.position).toBe(''); + expect(document.body.style.overflow).toBe(''); }); it('should block the scroll when modal is rendered open', () => { @@ -114,7 +114,7 @@ describe('modal', () => {
modal content
); - expect(document.documentElement.style.position).toBe('fixed'); + expect(document.body.style.overflow).toBe('hidden'); }); it('should block scroll when prop open change to true', () => { @@ -123,14 +123,14 @@ describe('modal', () => {
modal content
); - expect(document.documentElement.style.position).toBe(''); + expect(document.body.style.overflow).toBe(''); rerender( null}>
modal content
); - expect(document.documentElement.style.position).toBe('fixed'); + expect(document.body.style.overflow).toBe('hidden'); }); it('should unblock scroll when prop open change to false', async () => { @@ -139,7 +139,7 @@ describe('modal', () => {
modal content
); - expect(document.documentElement.style.position).toBe('fixed'); + expect(document.body.style.overflow).toBe('hidden'); rerender( null} animationDuration={0}> @@ -155,7 +155,7 @@ describe('modal', () => { { timeout: 1 } ); - expect(document.documentElement.style.position).toBe(''); + expect(document.body.style.overflow).toBe(''); }); it('should unblock scroll when unmounted directly', async () => { @@ -164,10 +164,10 @@ describe('modal', () => {
modal content
); - expect(document.documentElement.style.position).toBe('fixed'); + expect(document.body.style.overflow).toBe('hidden'); unmount(); - expect(document.documentElement.style.position).toBe(''); + expect(document.body.style.overflow).toBe(''); }); it('should unblock scroll when multiple modals are opened and then closed', async () => { @@ -181,7 +181,7 @@ describe('modal', () => { ); - expect(document.documentElement.style.position).toBe('fixed'); + expect(document.body.style.overflow).toBe('hidden'); // We close one modal, the scroll should be locked rerender( @@ -202,7 +202,7 @@ describe('modal', () => { }, { timeout: 1 } ); - expect(document.documentElement.style.position).toBe('fixed'); + expect(document.body.style.overflow).toBe('hidden'); // We close the second modal, the scroll should be unlocked rerender( @@ -223,7 +223,7 @@ describe('modal', () => { }, { timeout: 1 } ); - expect(document.documentElement.style.position).toBe(''); + expect(document.body.style.overflow).toBe(''); }); it('should unblock scroll when one modal is closed and the one still open has blockScroll set to false', async () => { @@ -237,7 +237,7 @@ describe('modal', () => { ); - expect(document.documentElement.style.position).toBe('fixed'); + expect(document.body.style.overflow).toBe('hidden'); // We close one modal, the scroll should be unlocked as remaining modal is not locking the scroll rerender( @@ -258,7 +258,7 @@ describe('modal', () => { }, { timeout: 1 } ); - expect(document.documentElement.style.position).toBe(''); + expect(document.body.style.overflow).toBe(''); }); }); @@ -437,7 +437,7 @@ describe('modal', () => {
modal content
); - expect(document.documentElement.style.position).toBe(''); + expect(document.body.style.overflow).toBe(''); }); }); diff --git a/react-responsive-modal/cypress/integration/modal.spec.ts b/react-responsive-modal/cypress/integration/modal.spec.ts index a8f7184d..89a00076 100644 --- a/react-responsive-modal/cypress/integration/modal.spec.ts +++ b/react-responsive-modal/cypress/integration/modal.spec.ts @@ -43,26 +43,26 @@ describe('simple modal', () => { it('should block the scroll when modal is opened', () => { cy.get('button').eq(0).click(); - cy.get('html').should('have.css', 'position', 'fixed'); + cy.get('body').should('have.css', 'overflow', 'hidden'); }); it('should unblock the scroll when modal is closed', () => { cy.get('button').eq(0).click(); - cy.get('html').should('have.css', 'position', 'fixed'); + cy.get('body').should('have.css', 'overflow', 'hidden'); cy.get('body').type('{esc}'); - cy.get('html').should('not.have.css', 'position', 'fixed'); + cy.get('body').should('not.have.css', 'overflow', 'hidden'); }); it('should unblock scroll only after last modal is closed when multiple modals are opened', () => { cy.get('button').eq(1).click(); cy.get('[data-testid=modal] button').eq(0).click(); cy.get('[data-testid=modal]').should('have.length', 2); - cy.get('html').should('have.css', 'position', 'fixed'); + cy.get('body').should('have.css', 'overflow', 'hidden'); cy.get('body').type('{esc}'); cy.get('[data-testid=modal]').should('have.length', 1); - cy.get('html').should('have.css', 'position', 'fixed'); + cy.get('body').should('have.css', 'overflow', 'hidden'); cy.get('body').type('{esc}'); cy.get('[data-testid=modal]').should('not.exist'); - cy.get('html').should('not.have.css', 'position', 'fixed'); + cy.get('body').should('not.have.css', 'overflow', 'hidden'); }); }); diff --git a/react-responsive-modal/package.json b/react-responsive-modal/package.json index b4e831e9..6efaf087 100644 --- a/react-responsive-modal/package.json +++ b/react-responsive-modal/package.json @@ -47,16 +47,16 @@ "size-limit": [ { "path": "dist/react-responsive-modal.cjs.production.min.js", - "limit": "3.3 KB" + "limit": "3.8 KB" }, { "path": "dist/react-responsive-modal.esm.js", - "limit": "3.3 KB" + "limit": "3.8 KB" } ], "dependencies": { - "classnames": "^2.2.6", - "no-scroll": "^2.1.1" + "body-scroll-lock": "^3.1.5", + "classnames": "^2.2.6" }, "peerDependencies": { "react": "^16.8.0 || ^17", @@ -66,6 +66,7 @@ "@size-limit/preset-small-lib": "4.7.0", "@testing-library/jest-dom": "5.11.6", "@testing-library/react": "11.1.2", + "@types/body-scroll-lock": "2.6.1", "@types/classnames": "2.2.11", "@types/no-scroll": "2.1.0", "@types/node": "14.14.7", diff --git a/react-responsive-modal/src/index.tsx b/react-responsive-modal/src/index.tsx index 0469fc67..37c23037 100644 --- a/react-responsive-modal/src/index.tsx +++ b/react-responsive-modal/src/index.tsx @@ -4,7 +4,8 @@ import cx from 'classnames'; import CloseIcon from './CloseIcon'; import { FocusTrap } from './FocusTrap'; import { modalManager, useModalManager } from './modalManager'; -import { isBrowser, blockNoScroll, unblockNoScroll } from './utils'; +import { useScrollLock } from './useScrollLock'; +import { isBrowser } from './utils'; const classes = { root: 'react-responsive-modal-root', @@ -183,13 +184,12 @@ export const Modal = ({ const [showPortal, setShowPortal] = useState(false); // Hook used to manage multiple modals opened at the same time - useModalManager(refModal, open, blockScroll); + useModalManager(refModal, open); - const handleOpen = () => { - if (blockScroll) { - blockNoScroll(); - } + // Hook used to manage the scroll + useScrollLock(refModal, open, showPortal, blockScroll); + const handleOpen = () => { if ( refContainer.current && !container && @@ -197,19 +197,11 @@ export const Modal = ({ ) { document.body.appendChild(refContainer.current); } + document.addEventListener('keydown', handleKeydown); }; const handleClose = () => { - // Restore the scroll only if there is no modal on the screen - // We filter the modals that are not affecting the scroll - if ( - blockScroll && - modalManager.modals().filter((modal) => modal.blockScroll).length === 0 - ) { - unblockNoScroll(); - } - if ( refContainer.current && !container && @@ -235,8 +227,8 @@ export const Modal = ({ useEffect(() => { return () => { - // When the component is unmounted directly we want to unblock the scroll if (showPortal) { + // When the modal is closed or removed directly, cleanup the listeners handleClose(); } }; diff --git a/react-responsive-modal/src/modalManager.ts b/react-responsive-modal/src/modalManager.ts index 6c02b92d..0733c45d 100644 --- a/react-responsive-modal/src/modalManager.ts +++ b/react-responsive-modal/src/modalManager.ts @@ -1,46 +1,37 @@ import { Ref, useEffect } from 'react'; -let modals: { element: Ref; blockScroll: boolean }[] = []; +let modals: Ref[] = []; /** * Handle the order of the modals. * Inspired by the material-ui implementation. */ export const modalManager = { - /** - * Return the modals array - */ - modals: () => modals, - /** * Register a new modal */ - add: (newModal: Ref, blockScroll: boolean) => { - modals.push({ element: newModal, blockScroll }); + add: (newModal: Ref) => { + modals.push(newModal); }, /** * Remove a modal */ - remove: (oldModal: Ref) => { - modals = modals.filter((modal) => modal.element !== oldModal); + remove: (oldModal: Ref) => { + modals = modals.filter((modal) => modal !== oldModal); }, /** * When multiple modals are rendered will return true if current modal is the last one */ - isTopModal: (modal: Ref) => - !!modals.length && modals[modals.length - 1].element === modal, + isTopModal: (modal: Ref) => + !!modals.length && modals[modals.length - 1] === modal, }; -export function useModalManager( - ref: Ref, - open: boolean, - blockScroll: boolean -) { +export function useModalManager(ref: Ref, open: boolean) { useEffect(() => { if (open) { - modalManager.add(ref, blockScroll); + modalManager.add(ref); } return () => { modalManager.remove(ref); diff --git a/react-responsive-modal/src/useScrollLock.ts b/react-responsive-modal/src/useScrollLock.ts new file mode 100644 index 00000000..810905c1 --- /dev/null +++ b/react-responsive-modal/src/useScrollLock.ts @@ -0,0 +1,24 @@ +import { useEffect, useRef } from 'react'; +import { disableBodyScroll, enableBodyScroll } from 'body-scroll-lock'; + +export const useScrollLock = ( + refModal: React.RefObject, + open: boolean, + showPortal: boolean, + blockScroll: boolean +) => { + const oldRef = useRef(null); + + useEffect(() => { + if (open && refModal.current && blockScroll) { + oldRef.current = refModal.current; + disableBodyScroll(refModal.current); + } + return () => { + if (oldRef.current) { + enableBodyScroll(oldRef.current); + oldRef.current = null; + } + }; + }, [open, showPortal, refModal]); +}; diff --git a/react-responsive-modal/src/utils.ts b/react-responsive-modal/src/utils.ts index d52f4143..aad96e76 100644 --- a/react-responsive-modal/src/utils.ts +++ b/react-responsive-modal/src/utils.ts @@ -1,11 +1 @@ -import noScroll from 'no-scroll'; - export const isBrowser = typeof window !== 'undefined'; - -export const blockNoScroll = () => { - noScroll.on(); -}; - -export const unblockNoScroll = () => { - noScroll.off(); -}; diff --git a/yarn.lock b/yarn.lock index 1c6d659c..18a06d5e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2410,6 +2410,13 @@ __metadata: languageName: node linkType: hard +"@types/body-scroll-lock@npm:2.6.1": + version: 2.6.1 + resolution: "@types/body-scroll-lock@npm:2.6.1" + checksum: 7cb4ed5ce6b9d927321e966970b67dfc6cc23836c2eb6b03e0ef6cc4713199ab0f9cbf5a9b56987939b63a0a9e8a2c5678c1d73808e04578446dc3ea24998e32 + languageName: node + linkType: hard + "@types/classnames@npm:2.2.11": version: 2.2.11 resolution: "@types/classnames@npm:2.2.11" @@ -3913,6 +3920,13 @@ __metadata: languageName: node linkType: hard +"body-scroll-lock@npm:^3.1.5": + version: 3.1.5 + resolution: "body-scroll-lock@npm:3.1.5" + checksum: e8de58edc0fd7d483e3971045ff83fe6d722592d5153e9bfc9da2962a179a6522b432a4b35344bc8ae522eec080cc87301ce4ded4878f26254bf42a4f26d27ed + languageName: node + linkType: hard + "boolbase@npm:^1.0.0, boolbase@npm:~1.0.0": version: 1.0.0 resolution: "boolbase@npm:1.0.0" @@ -10615,13 +10629,6 @@ fsevents@^1.2.7: languageName: node linkType: hard -"no-scroll@npm:^2.1.1": - version: 2.1.1 - resolution: "no-scroll@npm:2.1.1" - checksum: a575d5d3b84164a3eab41f4854526bf66969428533fccba87bd9898a86123cdf625c7bfb26bf1b479698dfec9bcf2f17ae9c743f196ca9469b6296e2e13f4b8e - languageName: node - linkType: hard - "node-abi@npm:^2.7.0": version: 2.19.1 resolution: "node-abi@npm:2.19.1" @@ -12511,6 +12518,7 @@ fsevents@^1.2.7: "@size-limit/preset-small-lib": 4.7.0 "@testing-library/jest-dom": 5.11.6 "@testing-library/react": 11.1.2 + "@types/body-scroll-lock": 2.6.1 "@types/classnames": 2.2.11 "@types/no-scroll": 2.1.0 "@types/node": 14.14.7 @@ -12518,10 +12526,10 @@ fsevents@^1.2.7: "@types/react-dom": 16.9.9 "@types/react-transition-group": 4.4.0 babel-jest: 26.6.3 + body-scroll-lock: ^3.1.5 classnames: ^2.2.6 cypress: 5.6.0 husky: 4.3.0 - no-scroll: ^2.1.1 prettier: 2.1.2 react: 17.0.1 react-dom: 17.0.1