From 6ebeb116d375517c6e1785cdba5a770f9bd01ce4 Mon Sep 17 00:00:00 2001 From: Nathan Vasse Date: Thu, 4 Apr 2024 15:01:29 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(react)=20migrate=20Modals=20?= =?UTF-8?q?to=20react=20modal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We encoutered an issue where stacked modal backdrop were not rendered above the modal below. It was caused by the dialog element that is natively rendered on the top layer regardless where it is create in the DOM. So we decided to use react modal that provides hand crafted dialog, and implementing a11y features. Closes #314 --- .changeset/heavy-coats-beam.md | 5 + packages/react/package.json | 2 + .../src/components/Modal/ModalProvider.tsx | 27 +--- .../react/src/components/Modal/index.scss | 12 ++ .../react/src/components/Modal/index.spec.tsx | 40 ++++-- packages/react/src/components/Modal/index.tsx | 117 ++++++------------ .../react/src/components/Provider/index.tsx | 4 +- yarn.lock | 36 +++++- 8 files changed, 128 insertions(+), 115 deletions(-) create mode 100644 .changeset/heavy-coats-beam.md diff --git a/.changeset/heavy-coats-beam.md b/.changeset/heavy-coats-beam.md new file mode 100644 index 0000000..f9b05c4 --- /dev/null +++ b/.changeset/heavy-coats-beam.md @@ -0,0 +1,5 @@ +--- +"@openfun/cunningham-react": minor +--- + +migrate Modals to react modal diff --git a/packages/react/package.json b/packages/react/package.json index 680b906..1301de9 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -52,6 +52,7 @@ "@react-stately/calendar": "3.4.4", "@react-stately/datepicker": "3.9.2", "@tanstack/react-table": "8.13.2", + "@types/react-modal": "3.16.3", "chromatic": "11.2.0", "classnames": "2.5.1", "downshift": "8.4.0", @@ -59,6 +60,7 @@ "react-aria": "3.32.1", "react-aria-components": "1.1.1", "react-dom": "18.2.0", + "react-modal": "3.16.1", "react-stately": "3.30.1" }, "engines": { diff --git a/packages/react/src/components/Modal/ModalProvider.tsx b/packages/react/src/components/Modal/ModalProvider.tsx index 5a13a47..f9dab63 100644 --- a/packages/react/src/components/Modal/ModalProvider.tsx +++ b/packages/react/src/components/Modal/ModalProvider.tsx @@ -10,6 +10,7 @@ import React, { useMemo, useState, } from "react"; +import ReactModal from "react-modal"; import { DeleteConfirmationModal, DeleteConfirmationModalProps, @@ -104,6 +105,10 @@ type ModalMap = Map; export const ModalProvider = ({ children }: PropsWithChildren) => { const [modals, setModals] = useState({} as ModalMap); + useEffect(() => { + ReactModal.setAppElement(".c__app"); + }, []); + const addModal = ( component: FunctionComponent, props: Partial, @@ -150,28 +155,6 @@ 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 9f16473..95c7dd3 100644 --- a/packages/react/src/components/Modal/index.scss +++ b/packages/react/src/components/Modal/index.scss @@ -8,6 +8,17 @@ box-shadow: var(--c--components--modal--box-shadow); color: var(--c--components--modal--color); box-sizing: border-box; + position: fixed; + left: 50%; + top: 50%; + transform: translate(-50%, -50%); + max-width: calc(100% - 2em - 6px); + max-height: calc(100% - 2em - 6px); + overflow: auto; + + &:focus-visible { + outline: none; + } &::backdrop { // ::backdrop does not inherit from its element so CSS vars does not work. @@ -181,6 +192,7 @@ border: none; display: flex; flex-direction: column; + transform: none; .c__modal__content { flex-grow: 1; diff --git a/packages/react/src/components/Modal/index.spec.tsx b/packages/react/src/components/Modal/index.spec.tsx index 35b528f..26e089b 100644 --- a/packages/react/src/components/Modal/index.spec.tsx +++ b/packages/react/src/components/Modal/index.spec.tsx @@ -22,7 +22,6 @@ describe("", () => { render(); expect(await screen.findByText("Modal Content")).toBeInTheDocument(); }); - it("shows a modal when clicking on the button", async () => { const Wrapper = () => { const modal = useModal(); @@ -44,6 +43,32 @@ describe("", () => { await user.click(button); expect(screen.getByText("Modal Content")).toBeInTheDocument(); }); + it("sets aria-hidden on app when a modal is opened", async () => { + const Wrapper = () => { + const modal = useModal(); + return ( + + + +
Modal Content
+ + + ); + }; + + render(); + const user = userEvent.setup(); + const button = screen.getByText("Open Modal"); + const app = document.querySelector(".c__app"); + + expect(screen.queryByText("Modal Content")).not.toBeInTheDocument(); + expect(app).not.toHaveAttribute("aria-hidden", "true"); + + await user.click(button); + + expect(screen.getByText("Modal Content")).toBeInTheDocument(); + expect(app).toHaveAttribute("aria-hidden", "true"); + }); it("closes the modal when clicking on the close button", async () => { const Wrapper = () => { const modal = useModal(); @@ -118,16 +143,7 @@ describe("", () => { await user.click(button); expect(screen.getByText("Modal Content")).toBeInTheDocument(); - const modal = screen.getByRole("dialog"); - // We move the pointer on the edge of the screen in order to simulate the click outside. - await user.pointer({ - coords: { - x: 1, - y: 1, - }, - }); - - await user.click(modal); + await user.click(document.querySelector(".c__modal__backdrop")!); expect(screen.queryByText("Modal Content")).not.toBeInTheDocument(); }); @@ -292,7 +308,6 @@ describe("", () => { // Display the modal. await user.click(button); - // Modal is open. expect(screen.getByText("Are you sure?")).toBeInTheDocument(); expect( @@ -452,6 +467,7 @@ describe("", () => { 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", { diff --git a/packages/react/src/components/Modal/index.tsx b/packages/react/src/components/Modal/index.tsx index ad6d40a..153a777 100644 --- a/packages/react/src/components/Modal/index.tsx +++ b/packages/react/src/components/Modal/index.tsx @@ -1,10 +1,13 @@ import React, { PropsWithChildren, ReactNode, useEffect } from "react"; import classNames from "classnames"; -import { createPortal } from "react-dom"; +import ReactModal from "react-modal"; import { Button } from ":/components/Button"; +import { NOSCROLL_CLASS } from ":/components/Modal/ModalProvider"; export type ModalHandle = {}; +export const MODAL_CLASS = "c__modal"; + export enum ModalSize { SMALL = "small", MEDIUM = "medium", @@ -69,91 +72,47 @@ export const Modal = (props: ModalProps) => { }; export const ModalInner = (props: ModalProps) => { - const ref = React.useRef(null); - - useEffect(() => { - if (props.isOpen) { - ref.current?.showModal(); - } else { - ref.current?.close(); - } - }, [props.isOpen]); - - useEffect(() => { - ref.current?.addEventListener("close", () => props.onClose(), { - once: true, - }); - - const onClick = (event: MouseEvent) => { - const rect = ref.current!.getBoundingClientRect(); - const isInDialog = - rect.top <= event.clientY && - event.clientY <= rect.top + rect.height && - rect.left <= event.clientX && - event.clientX <= rect.left + rect.width; - if (!isInDialog) { - props.onClose(); - } - }; - - if (props.closeOnClickOutside) { - ref.current?.addEventListener("click", onClick); - } - - const preventClose = (event: Event) => { - event.preventDefault(); - }; - - if (props.preventClose) { - ref.current?.addEventListener("cancel", preventClose); - } - - return () => { - ref.current?.removeEventListener("click", onClick); - ref.current?.removeEventListener("click", preventClose); - }; - }, [props.isOpen]); + const { modalParentSelector } = useModals(); if (!props.isOpen) { return null; } return ( - <> - {createPortal( - <> -
- - {!props.hideCloseButton && !props.preventClose && ( -
-
- )} - {props.titleIcon && ( -
{props.titleIcon}
- )} - {props.title && ( -
{props.title}
- )} - - {/* eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex */} -
- {props.children} -
- - - , - document.getElementById("c__modals-portal")!, + { + if (!props.preventClose) { + props.onClose(); + } + }} + parentSelector={modalParentSelector} + overlayClassName="c__modal__backdrop" + className={classNames(MODAL_CLASS, `${MODAL_CLASS}--${props.size}`)} + shouldCloseOnOverlayClick={!!props.closeOnClickOutside} + bodyOpenClassName={classNames("c__modals--opened", NOSCROLL_CLASS)} + > + {!props.hideCloseButton && !props.preventClose && ( +
+
)} - + {props.titleIcon && ( +
{props.titleIcon}
+ )} + {props.title &&
{props.title}
} + + {/* eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex */} +
+ {props.children} +
+ + ); }; diff --git a/packages/react/src/components/Provider/index.tsx b/packages/react/src/components/Provider/index.tsx index af8288c..adaf2b9 100644 --- a/packages/react/src/components/Provider/index.tsx +++ b/packages/react/src/components/Provider/index.tsx @@ -103,7 +103,9 @@ export const CunninghamProvider = ({ return ( - {children} +
+ {children} +
); diff --git a/yarn.lock b/yarn.lock index 85c4e9a..871af31 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4346,6 +4346,13 @@ dependencies: "@types/react" "*" +"@types/react-modal@^3.16.3": + version "3.16.3" + resolved "https://registry.yarnpkg.com/@types/react-modal/-/react-modal-3.16.3.tgz#250f32c07f1de28e2bcf9c3e84b56adaa6897013" + integrity sha512-xXuGavyEGaFQDgBv4UVm8/ZsG+qxeQ7f77yNrW3n+1J6XAstUy5rYHeIHPh1KzsGc6IkCIdu6lQ2xWzu1jBTLg== + dependencies: + "@types/react" "*" + "@types/react@*", "@types/react@^16.8.0 || ^17.0.0 || ^18.0.0": version "18.2.66" resolved "https://registry.npmjs.org/@types/react/-/react-18.2.66.tgz" @@ -6913,6 +6920,11 @@ execa@^8.0.1: signal-exit "^4.1.0" strip-final-newline "^3.0.0" +exenv@^1.2.0: + version "1.2.2" + resolved "https://registry.yarnpkg.com/exenv/-/exenv-1.2.2.tgz#2ae78e85d9894158670b03d47bec1f03bd91bb9d" + integrity sha512-Z+ktTxTwv9ILfgKCk32OX3n/doe+OcLTRtqK9pcL+JsP3J1/VW8Uvl4ZjLlKqeW4rzK4oesDOGMEMRIZqtP4Iw== + exit@^0.1.2: version "0.1.2" resolved "https://registry.npmjs.org/exit/-/exit-0.1.2.tgz" @@ -8910,7 +8922,7 @@ longest-streak@^3.0.0: resolved "https://registry.npmjs.org/longest-streak/-/longest-streak-3.1.0.tgz" integrity sha512-9Ri+o0JYgehTaVBBDoMqIl8GXtbWg711O3srftcHhZ0dqnETqLaoIK0x17fUw9rFSlK/0NlsKe0Ahhyl5pXE2g== -loose-envify@^1.1.0, loose-envify@^1.4.0: +loose-envify@^1.0.0, loose-envify@^1.1.0, loose-envify@^1.4.0: version "1.4.0" resolved "https://registry.npmjs.org/loose-envify/-/loose-envify-1.4.0.tgz" integrity sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q== @@ -10645,6 +10657,21 @@ react-is@^18.0.0, react-is@^18.2.0: resolved "https://registry.npmjs.org/react-is/-/react-is-18.2.0.tgz" integrity sha512-xWGDIW6x921xtzPkhiULtthJHoJvBbF3q26fzloPCK0hsvxtPVelvftw3zjbHWSkR2km9Z+4uxbDDK/6Zw9B8w== +react-lifecycles-compat@^3.0.0: + version "3.0.4" + resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362" + integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA== + +react-modal@^3.16.1: + version "3.16.1" + resolved "https://registry.yarnpkg.com/react-modal/-/react-modal-3.16.1.tgz#34018528fc206561b1a5467fc3beeaddafb39b2b" + integrity sha512-VStHgI3BVcGo7OXczvnJN7yT2TWHJPDXZWyI/a0ssFNhGZWsPmB8cF0z33ewDXq4VfYMO1vXgiv/g8Nj9NDyWg== + dependencies: + exenv "^1.2.0" + prop-types "^15.7.2" + react-lifecycles-compat "^3.0.0" + warning "^4.0.3" + react-refresh@^0.14.0: version "0.14.0" resolved "https://registry.npmjs.org/react-refresh/-/react-refresh-0.14.0.tgz" @@ -12470,6 +12497,13 @@ walker@^1.0.8: dependencies: makeerror "1.0.12" +warning@^4.0.3: + version "4.0.3" + resolved "https://registry.yarnpkg.com/warning/-/warning-4.0.3.tgz#16e9e077eb8a86d6af7d64aa1e05fd85b4678ca3" + integrity sha512-rpJyN222KWIvHJ/F53XSZv0Zl/accqHR8et1kpaMTD/fLCRxtV8iX8czMzY7sVZupTI3zcUTg8eycS2kNF9l6w== + dependencies: + loose-envify "^1.0.0" + watchpack@^2.2.0: version "2.4.1" resolved "https://registry.npmjs.org/watchpack/-/watchpack-2.4.1.tgz"