Close context menu when a modal is opened to prevent user getting stu…

…ck (#9560)
matrix-org · Nov 9, 2022 · da77953 · da77953
1 parent 7fbdd8b
commit da77953
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 3 deletions.
diff --git 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<T extends any[]> {
 
 type ParametersWithoutFirst<T extends (...args: any) => 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<ModalManagerEvent, HandlerMap> {
     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<T> = {},
     ): IHandle<T> {
+        const beforeModal = this.getCurrentModal();
         const { modal, closeDialog, onFinishedProm } = this.buildModal<T>(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<T>,
         className?: string,
     ): IHandle<T> {
+        const beforeModal = this.getCurrentModal();
         const { modal, closeDialog, onFinishedProm } = this.buildModal<T>(prom, props, className, {});
 
         this.modals.push(modal);
+
         this.reRender();
+        this.emitIfChanged(beforeModal);
+
         return {
             close: closeDialog,
             finished: onFinishedProm,
         };
     }
 
+    private emitIfChanged(beforeModal?: IModal<any>): 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
@@ -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<IProps, IState> {
         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<IProps, IState> {
     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
@@ -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("<ContextMenu />", () => {
     // Hardcode window and menu dimensions
@@ -141,4 +143,45 @@ describe("<ContextMenu />", () => {
             expect(actualChevronOffset).toEqual(targetChevronOffset + targetX - actualX);
         });
     });
+
+    it("should automatically close when a modal is opened", () => {
+        const targetX = -50;
+        const onFinished = jest.fn();
+
+        mount(
+            <ContextMenu
+                top={0}
+                right={windowSize - targetX - menuSize}
+                chevronFace={ChevronFace.Bottom}
+                onFinished={onFinished}
+                chevronOffset={targetChevronOffset}
+            />,
+        );
+
+        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(
+            <ContextMenu
+                top={0}
+                right={windowSize - targetX - menuSize}
+                chevronFace={ChevronFace.Bottom}
+                onFinished={onFinished}
+                chevronOffset={targetChevronOffset}
+            />,
+        );
+
+        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
@@ -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('<LocationShareMenu />', () => {