From 7f12f4d9b0573656e3c1de18ce1e3d09388d3fdb Mon Sep 17 00:00:00 2001 From: Nathan Vasse Date: Mon, 19 Feb 2024 15:26:08 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(fix)=20fix=20body=20scroll=20when?= =?UTF-8?q?=20a=20modal=20is=20opened?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was possible to scroll the body when a modal is opened, it was cutting the visibility of the modal, and was simply weird. Fixes #263 --- .../components/Modal/EdgeCases.stories.tsx | 105 ++++++++++++++++++ .../src/components/Modal/ModalProvider.tsx | 25 +++++ .../react/src/components/Modal/index.scss | 4 + .../react/src/components/Modal/index.spec.tsx | 102 ++++++++++++++++- packages/react/src/hooks/usePrevious.ts | 19 ++++ 5 files changed, 254 insertions(+), 1 deletion(-) create mode 100644 packages/react/src/components/Modal/EdgeCases.stories.tsx create mode 100644 packages/react/src/hooks/usePrevious.ts diff --git a/packages/react/src/components/Modal/EdgeCases.stories.tsx b/packages/react/src/components/Modal/EdgeCases.stories.tsx new file mode 100644 index 0000000..75d3db5 --- /dev/null +++ b/packages/react/src/components/Modal/EdgeCases.stories.tsx @@ -0,0 +1,105 @@ +import React, { useEffect } from "react"; +import { faker } from "@faker-js/faker"; +import { Meta } from "@storybook/react"; +import { Button } from ":/components/Button"; +import { CunninghamProvider } from ":/components/Provider"; +import { Modal, ModalSize, useModal } from ":/components/Modal/index"; + +const meta: Meta = { + title: "Components/Modal/Edge Cases", + parameters: { + docs: { + story: { + height: "350px", + }, + }, + }, +}; + +export default meta; + +export const StackedModals = { + render: () => { + const modal1 = useModal(); + const modal2 = useModal(); + const modal3 = useModal(); + + const openModals = () => { + modal1.open(); + setTimeout(() => { + modal2.open(); + setTimeout(() => { + modal3.open(); + }, 100); + }, 100); + }; + + useEffect(() => { + openModals(); + }, []); + + const array = Array.from({ length: 1000 }, (_, i) => i); + + return ( + +
+ + Tertiary} + rightActions={ + <> + + + + } + size={ModalSize.LARGE} + {...modal1} + > + {faker.lorem.lines(400)} + + Tertiary} + rightActions={ + <> + + + + } + size={ModalSize.MEDIUM} + {...modal2} + > + {faker.lorem.lines(400)} + + Tertiary} + rightActions={ + <> + + + + } + size={ModalSize.SMALL} + {...modal3} + > + {faker.lorem.lines(400)} + + {array.map((i) => ( +
{i}
+ ))} +
+ + ); + }, +}; diff --git a/packages/react/src/components/Modal/ModalProvider.tsx b/packages/react/src/components/Modal/ModalProvider.tsx index 6451db4..5a13a47 100644 --- a/packages/react/src/components/Modal/ModalProvider.tsx +++ b/packages/react/src/components/Modal/ModalProvider.tsx @@ -6,6 +6,7 @@ import React, { PropsWithChildren, ReactNode, useContext, + useEffect, useMemo, useState, } from "react"; @@ -25,6 +26,8 @@ import { MessageModalProps, } from ":/components/Modal/MessageModal"; +export const NOSCROLL_CLASS = "c__noscroll"; + export type Decision = string | null | undefined; export type DecisionModalProps = WithOptional & { onDecide: (decision?: Decision) => void; @@ -147,6 +150,28 @@ export const ModalProvider = ({ children }: PropsWithChildren) => { [], ); + useEffect(() => { + const portalElement = document.getElementById("c__modals-portal")!; + // Create an observer instance linked to the callback function + const observer = new MutationObserver(() => { + const dialogs = portalElement.querySelectorAll("dialog"); + if (dialogs.length > 0) { + document.querySelector("body")!.classList.add(NOSCROLL_CLASS); + } else { + document.querySelector("body")!.classList.remove(NOSCROLL_CLASS); + } + }); + + // Start observing the target node for configured mutations + observer.observe(portalElement, { + childList: true, + }); + + return () => { + observer.disconnect(); + }; + }, []); + return ( {children} diff --git a/packages/react/src/components/Modal/index.scss b/packages/react/src/components/Modal/index.scss index b209274..d7e3183 100644 --- a/packages/react/src/components/Modal/index.scss +++ b/packages/react/src/components/Modal/index.scss @@ -153,3 +153,7 @@ } } } + +.c__noscroll { + overflow: hidden; +} diff --git a/packages/react/src/components/Modal/index.spec.tsx b/packages/react/src/components/Modal/index.spec.tsx index 62e45f8..35b528f 100644 --- a/packages/react/src/components/Modal/index.spec.tsx +++ b/packages/react/src/components/Modal/index.spec.tsx @@ -3,7 +3,7 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { Modal, ModalSize, useModal } from ":/components/Modal/index"; import { CunninghamProvider } from ":/components/Provider"; -import { useModals } from ":/components/Modal/ModalProvider"; +import { NOSCROLL_CLASS, useModals } from ":/components/Modal/ModalProvider"; import { VariantType } from ":/utils/VariantUtils"; describe("", () => { @@ -432,4 +432,104 @@ describe("", () => { // Decision is undefined. expect(decision).toBeUndefined(); }); + + it("sets a noscroll class to body when a modal is open and remove it on close", async () => { + const Wrapper = () => { + const modal = useModal(); + return ( + + + +
Modal Content
+ + + ); + }; + + render(); + const user = userEvent.setup(); + const button = screen.getByText("Open Modal"); + + expect(document.body.classList.contains(NOSCROLL_CLASS)).toBeFalsy(); + await user.click(button); + expect(document.body.classList.contains(NOSCROLL_CLASS)).toBeTruthy(); + + const closeButton = screen.getByRole("button", { + name: "close", + }); + await user.click(closeButton); + expect(document.body.classList.contains(NOSCROLL_CLASS)).toBeFalsy(); + }); + + it("removes noscroll body class only when last modal is closed when multiple are opened", async () => { + const Wrapper = () => { + const modal1 = useModal(); + const modal2 = useModal(); + const modal3 = useModal(); + const openModals = () => { + modal1.open(); + modal2.open(); + modal3.open(); + }; + return ( + + + + Modal 1 + + + Modal 2 + + + Modal 3 + + + ); + }; + + render(); + const user = userEvent.setup(); + const button = screen.getByText("Open Modals"); + + expect(document.body.classList.contains(NOSCROLL_CLASS)).toBeFalsy(); + await user.click(button); + + expect(screen.getByText("Modal 1")).toBeInTheDocument(); + expect(screen.getByText("Modal 2")).toBeInTheDocument(); + expect(screen.getByText("Modal 3")).toBeInTheDocument(); + + expect(document.body.classList.contains(NOSCROLL_CLASS)).toBeTruthy(); + + const closeButtons = screen.getAllByRole("button", { + name: "close", + }); + expect(closeButtons).toHaveLength(3); + + // Close modal 1. + await user.click(closeButtons[0]); + expect(screen.queryByText("Modal 1")).not.toBeInTheDocument(); + expect(screen.getByText("Modal 2")).toBeInTheDocument(); + expect(screen.getByText("Modal 3")).toBeInTheDocument(); + + // class is still on body. + expect(document.body.classList.contains(NOSCROLL_CLASS)).toBeTruthy(); + + // Close modal 2. + await user.click(closeButtons[1]); + expect(screen.queryByText("Modal 1")).not.toBeInTheDocument(); + expect(screen.queryByText("Modal 2")).not.toBeInTheDocument(); + expect(screen.getByText("Modal 3")).toBeInTheDocument(); + + // class is still on body. + expect(document.body.classList.contains(NOSCROLL_CLASS)).toBeTruthy(); + + // Close modal 3. + await user.click(closeButtons[2]); + expect(screen.queryByText("Modal 1")).not.toBeInTheDocument(); + expect(screen.queryByText("Modal 2")).not.toBeInTheDocument(); + expect(screen.queryByText("Modal 3")).not.toBeInTheDocument(); + + // class is removed from body. + expect(document.body.classList.contains(NOSCROLL_CLASS)).toBeFalsy(); + }); }); diff --git a/packages/react/src/hooks/usePrevious.ts b/packages/react/src/hooks/usePrevious.ts new file mode 100644 index 0000000..aa8026a --- /dev/null +++ b/packages/react/src/hooks/usePrevious.ts @@ -0,0 +1,19 @@ +import { useEffect, useRef } from "react"; + +/** + * Hook which stores the previous value of a component prop or state. + * https://usehooks.com/usePrevious/ + * + * @param value + */ +const usePrevious = (value: T): T => { + const previous = useRef(value); + + useEffect(() => { + previous.current = value; + }, [value]); + + return previous.current; +}; + +export default usePrevious;