From 9548dc705a7734191cd4e91af3ab7512e25651a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Thu, 1 Aug 2019 20:02:39 +0100 Subject: [PATCH] fix(ui): don't reset scroll lock on hidden modal update --- ui/src/Components/Modal/index.js | 13 +++++++++++-- ui/src/Components/Modal/index.test.js | 24 +++++++++++++++++++----- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/ui/src/Components/Modal/index.js b/ui/src/Components/Modal/index.js index 7a66373bb..c23f37b76 100644 --- a/ui/src/Components/Modal/index.js +++ b/ui/src/Components/Modal/index.js @@ -3,6 +3,7 @@ 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"; @@ -29,6 +30,7 @@ const Modal = observer( super(props); this.modalRef = React.createRef(); this.HotKeysRef = React.createRef(); + this.lastIsOpen = observable.box(false); } toggleBodyClass = isOpen => { @@ -39,16 +41,23 @@ const Modal = observer( } else { clearAllBodyScrollLocks(); } + this.lastIsOpen.set(isOpen); }; componentDidMount() { const { isOpen } = this.props; - this.toggleBodyClass(isOpen); + if (isOpen) { + this.toggleBodyClass(isOpen); + } } componentDidUpdate() { const { isOpen } = this.props; - this.toggleBodyClass(isOpen); + // 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() { diff --git a/ui/src/Components/Modal/index.test.js b/ui/src/Components/Modal/index.test.js index 35e65fcd7..c48ea5fcd 100644 --- a/ui/src/Components/Modal/index.test.js +++ b/ui/src/Components/Modal/index.test.js @@ -24,17 +24,31 @@ describe("", () => { expect(document.body.className.split(" ")).toContain("modal-open"); }); - it("'modal-open' class is removed from body node after modal is hidden", () => { - MountedModal(false); - expect(document.body.className.split(" ")).not.toContain("modal-open"); - }); - it("'modal-open' class is removed from body node after modal is unmounted", () => { const tree = MountedModal(true); tree.unmount(); expect(document.body.className.split(" ")).not.toContain("modal-open"); }); + it("'modal-open' class is not removed from body when hidden modal is updated", () => { + document.body.classList.toggle("modal-open", true); + const tree = MountedModal(false); + expect(document.body.className.split(" ")).toContain("modal-open"); + tree.instance().componentDidUpdate(); + expect(document.body.className.split(" ")).toContain("modal-open"); + }); + + it("'modal-open' class is removed from body when visible modal is updated to be hidden", () => { + document.body.classList.toggle("modal-open", true); + let isOpen = true; + const tree = MountedModal(isOpen); + expect(document.body.className.split(" ")).toContain("modal-open"); + + isOpen = false; + tree.instance().componentDidUpdate(); + expect(document.body.className.split(" ")).toContain("modal-open"); + }); + it("passes extra props down to the MountModal animation component", () => { const onExited = jest.fn(); const tree = mount(