Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit da77953

Browse files
authored
Close context menu when a modal is opened to prevent user getting stuck (#9560)
1 parent 7fbdd8b commit da77953

File tree

4 files changed

+81
-3
lines changed

4 files changed

+81
-3
lines changed

src/Modal.tsx

+23-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import React from 'react';
1919
import ReactDOM from 'react-dom';
2020
import classNames from 'classnames';
2121
import { defer, sleep } from "matrix-js-sdk/src/utils";
22+
import { TypedEventEmitter } from 'matrix-js-sdk/src/models/typed-event-emitter';
2223

2324
import dis from './dispatcher/dispatcher';
2425
import AsyncWrapper from './AsyncWrapper';
@@ -54,7 +55,15 @@ interface IOptions<T extends any[]> {
5455

5556
type ParametersWithoutFirst<T extends (...args: any) => any> = T extends (a: any, ...args: infer P) => any ? P : never;
5657

57-
export class ModalManager {
58+
export enum ModalManagerEvent {
59+
Opened = "opened",
60+
}
61+
62+
type HandlerMap = {
63+
[ModalManagerEvent.Opened]: () => void;
64+
};
65+
66+
export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMap> {
5867
private counter = 0;
5968
// The modal to prioritise over all others. If this is set, only show
6069
// this modal. Remove all other modals from the stack when this modal
@@ -244,6 +253,7 @@ export class ModalManager {
244253
isStaticModal = false,
245254
options: IOptions<T> = {},
246255
): IHandle<T> {
256+
const beforeModal = this.getCurrentModal();
247257
const { modal, closeDialog, onFinishedProm } = this.buildModal<T>(prom, props, className, options);
248258
if (isPriorityModal) {
249259
// XXX: This is destructive
@@ -256,6 +266,8 @@ export class ModalManager {
256266
}
257267

258268
this.reRender();
269+
this.emitIfChanged(beforeModal);
270+
259271
return {
260272
close: closeDialog,
261273
finished: onFinishedProm,
@@ -267,16 +279,26 @@ export class ModalManager {
267279
props?: IProps<T>,
268280
className?: string,
269281
): IHandle<T> {
282+
const beforeModal = this.getCurrentModal();
270283
const { modal, closeDialog, onFinishedProm } = this.buildModal<T>(prom, props, className, {});
271284

272285
this.modals.push(modal);
286+
273287
this.reRender();
288+
this.emitIfChanged(beforeModal);
289+
274290
return {
275291
close: closeDialog,
276292
finished: onFinishedProm,
277293
};
278294
}
279295

296+
private emitIfChanged(beforeModal?: IModal<any>): void {
297+
if (beforeModal !== this.getCurrentModal()) {
298+
this.emit(ModalManagerEvent.Opened);
299+
}
300+
}
301+
280302
private onBackgroundClick = () => {
281303
const modal = this.getCurrentModal();
282304
if (!modal) {

src/components/structures/ContextMenu.tsx

+12-2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import UIStore from "../../stores/UIStore";
2626
import { checkInputableElement, RovingTabIndexProvider } from "../../accessibility/RovingTabIndex";
2727
import { KeyBindingAction } from "../../accessibility/KeyboardShortcuts";
2828
import { getKeyBindingsManager } from "../../KeyBindingsManager";
29+
import Modal, { ModalManagerEvent } from "../../Modal";
2930

3031
// Shamelessly ripped off Modal.js. There's probably a better way
3132
// 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> {
127128
this.initialFocus = document.activeElement as HTMLElement;
128129
}
129130

130-
componentWillUnmount() {
131+
public componentDidMount() {
132+
Modal.on(ModalManagerEvent.Opened, this.onModalOpen);
133+
}
134+
135+
public componentWillUnmount() {
136+
Modal.off(ModalManagerEvent.Opened, this.onModalOpen);
131137
// return focus to the thing which had it before us
132138
this.initialFocus.focus();
133139
}
134140

141+
private onModalOpen = () => {
142+
this.props.onFinished?.();
143+
};
144+
135145
private collectContextMenuRect = (element: HTMLDivElement) => {
136146
// We don't need to clean up when unmounting, so ignore
137147
if (!element) return;
@@ -183,7 +193,7 @@ export default class ContextMenu extends React.PureComponent<IProps, IState> {
183193
private onFinished = (ev: React.MouseEvent) => {
184194
ev.stopPropagation();
185195
ev.preventDefault();
186-
if (this.props.onFinished) this.props.onFinished();
196+
this.props.onFinished?.();
187197
};
188198

189199
private onClick = (ev: React.MouseEvent) => {

test/components/views/context_menus/ContextMenu-test.tsx

+43
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import { mount } from "enzyme";
2020

2121
import ContextMenu, { ChevronFace } from "../../../../src/components/structures/ContextMenu";
2222
import UIStore from "../../../../src/stores/UIStore";
23+
import Modal from "../../../../src/Modal";
24+
import BaseDialog from "../../../../src/components/views/dialogs/BaseDialog";
2325

2426
describe("<ContextMenu />", () => {
2527
// Hardcode window and menu dimensions
@@ -141,4 +143,45 @@ describe("<ContextMenu />", () => {
141143
expect(actualChevronOffset).toEqual(targetChevronOffset + targetX - actualX);
142144
});
143145
});
146+
147+
it("should automatically close when a modal is opened", () => {
148+
const targetX = -50;
149+
const onFinished = jest.fn();
150+
151+
mount(
152+
<ContextMenu
153+
top={0}
154+
right={windowSize - targetX - menuSize}
155+
chevronFace={ChevronFace.Bottom}
156+
onFinished={onFinished}
157+
chevronOffset={targetChevronOffset}
158+
/>,
159+
);
160+
161+
expect(onFinished).not.toHaveBeenCalled();
162+
Modal.createDialog(BaseDialog);
163+
expect(onFinished).toHaveBeenCalled();
164+
});
165+
166+
it("should not automatically close when a modal is opened under the existing one", () => {
167+
const targetX = -50;
168+
const onFinished = jest.fn();
169+
170+
Modal.createDialog(BaseDialog);
171+
mount(
172+
<ContextMenu
173+
top={0}
174+
right={windowSize - targetX - menuSize}
175+
chevronFace={ChevronFace.Bottom}
176+
onFinished={onFinished}
177+
chevronOffset={targetChevronOffset}
178+
/>,
179+
);
180+
181+
expect(onFinished).not.toHaveBeenCalled();
182+
Modal.createDialog(BaseDialog, {}, "", false, true);
183+
expect(onFinished).not.toHaveBeenCalled();
184+
Modal.appendDialog(BaseDialog);
185+
expect(onFinished).not.toHaveBeenCalled();
186+
});
144187
});

test/components/views/location/LocationShareMenu-test.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ jest.mock('../../../../src/stores/OwnProfileStore', () => ({
6969

7070
jest.mock('../../../../src/Modal', () => ({
7171
createDialog: jest.fn(),
72+
on: jest.fn(),
73+
off: jest.fn(),
74+
ModalManagerEvent: { Opened: "opened" },
7275
}));
7376

7477
describe('<LocationShareMenu />', () => {

0 commit comments

Comments
 (0)