From da779531f1a01696922384e7a64c9bf2dcd095df Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 9 Nov 2022 15:33:09 +0000 Subject: [PATCH] Close context menu when a modal is opened to prevent user getting stuck (#9560) --- src/Modal.tsx | 24 ++++++++++- src/components/structures/ContextMenu.tsx | 14 +++++- .../views/context_menus/ContextMenu-test.tsx | 43 +++++++++++++++++++ .../views/location/LocationShareMenu-test.tsx | 3 ++ 4 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/Modal.tsx b/src/Modal.tsx index 465f3cdad2..ee24b15d54 100644 --- a/src/Modal.tsx +++ b/src/Modal.tsx @@ -19,6 +19,7 @@ import React from 'react'; import ReactDOM from 'react-dom'; import classNames from 'classnames'; import { defer, sleep } from "matrix-js-sdk/src/utils"; +import { TypedEventEmitter } from 'matrix-js-sdk/src/models/typed-event-emitter'; import dis from './dispatcher/dispatcher'; import AsyncWrapper from './AsyncWrapper'; @@ -54,7 +55,15 @@ interface IOptions { type ParametersWithoutFirst any> = T extends (a: any, ...args: infer P) => any ? P : never; -export class ModalManager { +export enum ModalManagerEvent { + Opened = "opened", +} + +type HandlerMap = { + [ModalManagerEvent.Opened]: () => void; +}; + +export class ModalManager extends TypedEventEmitter { private counter = 0; // The modal to prioritise over all others. If this is set, only show // this modal. Remove all other modals from the stack when this modal @@ -244,6 +253,7 @@ export class ModalManager { isStaticModal = false, options: IOptions = {}, ): IHandle { + const beforeModal = this.getCurrentModal(); const { modal, closeDialog, onFinishedProm } = this.buildModal(prom, props, className, options); if (isPriorityModal) { // XXX: This is destructive @@ -256,6 +266,8 @@ export class ModalManager { } this.reRender(); + this.emitIfChanged(beforeModal); + return { close: closeDialog, finished: onFinishedProm, @@ -267,16 +279,26 @@ export class ModalManager { props?: IProps, className?: string, ): IHandle { + const beforeModal = this.getCurrentModal(); const { modal, closeDialog, onFinishedProm } = this.buildModal(prom, props, className, {}); this.modals.push(modal); + this.reRender(); + this.emitIfChanged(beforeModal); + return { close: closeDialog, finished: onFinishedProm, }; } + private emitIfChanged(beforeModal?: IModal): void { + if (beforeModal !== this.getCurrentModal()) { + this.emit(ModalManagerEvent.Opened); + } + } + private onBackgroundClick = () => { const modal = this.getCurrentModal(); if (!modal) { diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 136f036f70..cf9aacb808 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -26,6 +26,7 @@ import UIStore from "../../stores/UIStore"; import { checkInputableElement, RovingTabIndexProvider } from "../../accessibility/RovingTabIndex"; import { KeyBindingAction } from "../../accessibility/KeyboardShortcuts"; import { getKeyBindingsManager } from "../../KeyBindingsManager"; +import Modal, { ModalManagerEvent } from "../../Modal"; // Shamelessly ripped off Modal.js. There's probably a better way // of doing reusable widgets like dialog boxes & menus where we go and @@ -127,11 +128,20 @@ export default class ContextMenu extends React.PureComponent { this.initialFocus = document.activeElement as HTMLElement; } - componentWillUnmount() { + public componentDidMount() { + Modal.on(ModalManagerEvent.Opened, this.onModalOpen); + } + + public componentWillUnmount() { + Modal.off(ModalManagerEvent.Opened, this.onModalOpen); // return focus to the thing which had it before us this.initialFocus.focus(); } + private onModalOpen = () => { + this.props.onFinished?.(); + }; + private collectContextMenuRect = (element: HTMLDivElement) => { // We don't need to clean up when unmounting, so ignore if (!element) return; @@ -183,7 +193,7 @@ export default class ContextMenu extends React.PureComponent { private onFinished = (ev: React.MouseEvent) => { ev.stopPropagation(); ev.preventDefault(); - if (this.props.onFinished) this.props.onFinished(); + this.props.onFinished?.(); }; private onClick = (ev: React.MouseEvent) => { diff --git a/test/components/views/context_menus/ContextMenu-test.tsx b/test/components/views/context_menus/ContextMenu-test.tsx index 70ee4b95f4..3344dee48a 100644 --- a/test/components/views/context_menus/ContextMenu-test.tsx +++ b/test/components/views/context_menus/ContextMenu-test.tsx @@ -20,6 +20,8 @@ import { mount } from "enzyme"; import ContextMenu, { ChevronFace } from "../../../../src/components/structures/ContextMenu"; import UIStore from "../../../../src/stores/UIStore"; +import Modal from "../../../../src/Modal"; +import BaseDialog from "../../../../src/components/views/dialogs/BaseDialog"; describe("", () => { // Hardcode window and menu dimensions @@ -141,4 +143,45 @@ describe("", () => { expect(actualChevronOffset).toEqual(targetChevronOffset + targetX - actualX); }); }); + + it("should automatically close when a modal is opened", () => { + const targetX = -50; + const onFinished = jest.fn(); + + mount( + , + ); + + expect(onFinished).not.toHaveBeenCalled(); + Modal.createDialog(BaseDialog); + expect(onFinished).toHaveBeenCalled(); + }); + + it("should not automatically close when a modal is opened under the existing one", () => { + const targetX = -50; + const onFinished = jest.fn(); + + Modal.createDialog(BaseDialog); + mount( + , + ); + + expect(onFinished).not.toHaveBeenCalled(); + Modal.createDialog(BaseDialog, {}, "", false, true); + expect(onFinished).not.toHaveBeenCalled(); + Modal.appendDialog(BaseDialog); + expect(onFinished).not.toHaveBeenCalled(); + }); }); diff --git a/test/components/views/location/LocationShareMenu-test.tsx b/test/components/views/location/LocationShareMenu-test.tsx index 6609af93f3..f590bcdd7c 100644 --- a/test/components/views/location/LocationShareMenu-test.tsx +++ b/test/components/views/location/LocationShareMenu-test.tsx @@ -69,6 +69,9 @@ jest.mock('../../../../src/stores/OwnProfileStore', () => ({ jest.mock('../../../../src/Modal', () => ({ createDialog: jest.fn(), + on: jest.fn(), + off: jest.fn(), + ModalManagerEvent: { Opened: "opened" }, })); describe('', () => {