From 9dfe40c8379684c4c99d29f6c39d9d5065026e8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Tue, 9 Jun 2020 17:58:18 +0100 Subject: [PATCH] fix(ui): rewrite Modal with hooks --- ui/src/Components/MainModal/index.test.js | 5 + .../ManagedSilence/DeleteSilence.test.js | 1 + ui/src/Components/Modal/index.js | 168 +++++++----------- ui/src/Components/Modal/index.test.js | 16 +- ui/src/Components/OverviewModal/index.test.js | 13 +- ui/src/Components/SilenceModal/index.js | 8 +- ui/src/Components/SilenceModal/index.test.js | 8 + 7 files changed, 107 insertions(+), 112 deletions(-) diff --git a/ui/src/Components/MainModal/index.test.js b/ui/src/Components/MainModal/index.test.js index 3f007c55e..4c117bd09 100644 --- a/ui/src/Components/MainModal/index.test.js +++ b/ui/src/Components/MainModal/index.test.js @@ -34,6 +34,7 @@ beforeEach(() => { afterEach(() => { jest.restoreAllMocks(); fetchMock.reset(); + document.body.className = ""; }); const MountedMainModal = () => { @@ -111,8 +112,12 @@ describe("", () => { it("'modal-open' class is removed from body node after modal is hidden", () => { const tree = MountedMainModal(); const toggle = tree.find(".nav-link"); + toggle.simulate("click"); + expect(document.body.className.split(" ")).toContain("modal-open"); + toggle.simulate("click"); + act(() => jest.runOnlyPendingTimers()); expect(document.body.className.split(" ")).not.toContain("modal-open"); }); diff --git a/ui/src/Components/ManagedSilence/DeleteSilence.test.js b/ui/src/Components/ManagedSilence/DeleteSilence.test.js index 983ced60a..937b11a80 100644 --- a/ui/src/Components/ManagedSilence/DeleteSilence.test.js +++ b/ui/src/Components/ManagedSilence/DeleteSilence.test.js @@ -52,6 +52,7 @@ afterEach(() => { useFetchGet.mockReset(); useFetchDelete.mockReset(); clear(); + document.body.className = ""; }); const MockOnHide = jest.fn(); diff --git a/ui/src/Components/Modal/index.js b/ui/src/Components/Modal/index.js index 0f9af1893..6e8ff5b71 100644 --- a/ui/src/Components/Modal/index.js +++ b/ui/src/Components/Modal/index.js @@ -1,10 +1,7 @@ -import React, { Component } from "react"; +import React, { useRef, useEffect, useCallback } from "react"; import ReactDOM from "react-dom"; import PropTypes from "prop-types"; -import { observer } from "mobx-react"; -import { observable } from "mobx"; - import { disableBodyScroll, clearAllBodyScrollLocks } from "body-scroll-lock"; import { HotKeys } from "react-hotkeys"; @@ -14,105 +11,76 @@ import { MountModalBackdrop, } from "Components/Animations/MountModal"; -const Modal = observer( - class Modal extends Component { - static propTypes = { - size: PropTypes.oneOf(["lg", "xl"]), - isOpen: PropTypes.bool.isRequired, - isUpper: PropTypes.bool, - toggleOpen: PropTypes.func.isRequired, - children: PropTypes.node.isRequired, +const ModalInner = ({ size, isUpper, toggleOpen, children }) => { + const ref = useRef(null); + + useEffect(() => { + document.body.classList.add("modal-open"); + disableBodyScroll(ref.current); + return () => { + document.body.classList.remove("modal-open"); + clearAllBodyScrollLocks(); }; - static defaultProps = { - size: "lg", - isUpper: false, + }, []); + + const onRemount = useCallback(() => { + document.body.classList.add("modal-open"); + disableBodyScroll(ref.current); + }, []); + + useEffect(() => { + window.addEventListener("remountModal", onRemount); + return () => { + window.removeEventListener("remountModal", onRemount); }; + }, [onRemount]); - constructor(props) { - super(props); - this.modalRef = React.createRef(); - this.HotKeysRef = React.createRef(); - this.lastIsOpen = observable.box(false); - } + return ( + r && r.focus()} + keyMap={{ CLOSE: "Escape" }} + handlers={{ CLOSE: toggleOpen }} + className="modal-open" + > +
+
+
{children}
+
+
+ + ); +}; - toggleBodyClass = (isOpen) => { - document.body.classList.toggle("modal-open", isOpen); - if (isOpen) { - this.HotKeysRef.current.focus(); - disableBodyScroll(this.modalRef.current); - } else { - clearAllBodyScrollLocks(); - } - this.lastIsOpen.set(isOpen); - }; - - componentDidMount() { - const { isOpen } = this.props; - if (isOpen) { - this.toggleBodyClass(isOpen); - } - } - - componentDidUpdate() { - const { isOpen } = this.props; - // we shouldn't update if modal is hidden and was hidden previously - // which can happen when the button gets re-rendered - if (this.lastIsOpen.get() === true || isOpen === true) { - this.toggleBodyClass(isOpen); - } - } - - componentWillUnmount() { - const { isOpen } = this.props; - - if (isOpen) { - this.toggleBodyClass(false); - } - } - - render() { - const { - size, - isOpen, - isUpper, - toggleOpen, - children, - ...props - } = this.props; - - return ReactDOM.createPortal( - - - -
-
-
{children}
-
-
- - - -
- - , - document.body - ); - } - } -); +const Modal = ({ size, isOpen, isUpper, toggleOpen, children, ...props }) => { + return ReactDOM.createPortal( + + + + {children} + + + +
+ + , + document.body + ); +}; +Modal.propTypes = { + size: PropTypes.oneOf(["lg", "xl"]), + isOpen: PropTypes.bool.isRequired, + isUpper: PropTypes.bool, + toggleOpen: PropTypes.func.isRequired, + children: PropTypes.node.isRequired, +}; +Modal.defaultProps = { + size: "lg", + isUpper: false, +}; export { Modal }; diff --git a/ui/src/Components/Modal/index.test.js b/ui/src/Components/Modal/index.test.js index e7a560848..828e46285 100644 --- a/ui/src/Components/Modal/index.test.js +++ b/ui/src/Components/Modal/index.test.js @@ -16,15 +16,22 @@ const MountedModal = (isOpen) => { afterEach(() => { jest.resetAllMocks(); + document.body.className = ""; }); describe("", () => { + it("'modal-open' class is appended to MountModal container", () => { + const tree = MountedModal(true); + expect(tree.find("div").at(0).hasClass("modal-open")).toBe(true); + }); + it("'modal-open' class is appended to body node when modal is visible", () => { MountedModal(true); expect(document.body.className.split(" ")).toContain("modal-open"); }); it("'modal-open' class is not removed from body node after hidden modal is unmounted", () => { + document.body.classList.add("modal-open"); const tree = MountedModal(false); tree.unmount(); expect(document.body.className.split(" ")).toContain("modal-open"); @@ -51,9 +58,7 @@ describe("", () => { const tree = MountedModal(isOpen); expect(document.body.className.split(" ")).toContain("modal-open"); - isOpen = false; - // force update - tree.setProps({ style: {} }); + tree.setProps({ isOpen: false }); expect(document.body.className.split(" ")).toContain("modal-open"); }); @@ -70,7 +75,10 @@ describe("", () => { it("toggleOpen is called after pressing 'esc'", () => { const tree = MountedModal(true); - tree.simulate("keyDown", { key: "Escape", keyCode: 27, which: 27 }); + tree + .find("div") + .at(0) + .simulate("keyDown", { key: "Escape", keyCode: 27, which: 27 }); expect(fakeToggle).toHaveBeenCalled(); }); }); diff --git a/ui/src/Components/OverviewModal/index.test.js b/ui/src/Components/OverviewModal/index.test.js index 18c250f8f..71ba77845 100644 --- a/ui/src/Components/OverviewModal/index.test.js +++ b/ui/src/Components/OverviewModal/index.test.js @@ -16,6 +16,10 @@ beforeEach(() => { alertStore = new AlertStore([]); }); +afterEach(() => { + document.body.className = ""; +}); + const MountedOverviewModal = () => { return mount(); }; @@ -80,9 +84,12 @@ describe("", () => { it("'modal-open' class is removed from body node after modal is hidden", () => { const tree = MountedOverviewModal(); - const toggle = tree.find("div.navbar-brand"); - toggle.simulate("click"); - toggle.simulate("click"); + + tree.find("div.navbar-brand").simulate("click"); + expect(document.body.className.split(" ")).toContain("modal-open"); + + tree.find("div.navbar-brand").simulate("click"); + act(() => jest.runOnlyPendingTimers()); expect(document.body.className.split(" ")).not.toContain("modal-open"); }); diff --git a/ui/src/Components/SilenceModal/index.js b/ui/src/Components/SilenceModal/index.js index 05c581fd7..3129dc00b 100644 --- a/ui/src/Components/SilenceModal/index.js +++ b/ui/src/Components/SilenceModal/index.js @@ -1,4 +1,4 @@ -import React, { useRef } from "react"; +import React from "react"; import PropTypes from "prop-types"; import { observer } from "mobx-react"; @@ -22,10 +22,9 @@ const SilenceModalContent = React.lazy(() => const SilenceModal = observer( ({ alertStore, silenceFormStore, settingsStore }) => { - const modalRef = useRef(); - const onDeleteModalClose = React.useCallback(() => { - modalRef.current.toggleBodyClass(true); + const event = new CustomEvent("remountModal"); + window.dispatchEvent(event); }, []); return ( @@ -46,7 +45,6 @@ const SilenceModal = observer( { silenceFormStore = new SilenceFormStore(); }); +afterEach(() => { + document.body.className = ""; +}); + const MountedSilenceModal = () => { return mount( ", () => { it("'modal-open' class is removed from body node after modal is hidden", () => { const tree = MountedSilenceModal(); const toggle = tree.find(".nav-link"); + toggle.simulate("click"); + expect(document.body.className.split(" ")).toContain("modal-open"); + toggle.simulate("click"); + act(() => jest.runOnlyPendingTimers()); expect(document.body.className.split(" ")).not.toContain("modal-open"); });