Merge pull request #3492 from grebaldi/bugfix/2925/dialog-click-outside

BUGFIX: Limit click-outside handling for dialogs to their semi-transparent overlay
neos · Jul 4, 2023 · 522c992 · 522c992
2 parents b248045 + 11735bb
commit 522c992
Show file tree

Hide file tree

Showing 9 changed files with 394 additions and 130 deletions.
diff --git a/package.json b/package.json
@@ -31,7 +31,6 @@
     "@types/moment": "^2.13.0",
     "@types/prop-types": "^15.7.3",
     "@types/react": "^16.9.17",
-    "@types/react-close-on-escape": "^3.0.0",
     "@types/react-dom": "^16.9.4",
     "@types/react-fontawesome": "^1.6.4",
     "@types/react-portal": "^4.0.1",
@@ -160,7 +159,6 @@
     "prop-types": "^15.5.10",
     "raf": "^3.4.1",
     "react": "^16.12.0",
-    "react-close-on-escape": "^3.0.0",
     "react-codemirror2": "^7.2.1",
     "react-collapse": "^5.0.1",
     "react-datetime": "^2.8.10",

diff --git a/packages/react-ui-components/package.json b/packages/react-ui-components/package.json
@@ -53,7 +53,6 @@
     "react-dom": "^16.0.0",
     "react-height": "^3.0.0",
     "react-motion": "^0.5.0",
-    "react-portal": "^4.2.0",
     "react-svg": "^11.1.2",
     "react-textarea-autosize": "^8.3.0"
   },

diff --git a/packages/react-ui-components/src/Dialog/DialogManager.spec.ts b/packages/react-ui-components/src/Dialog/DialogManager.spec.ts
@@ -0,0 +1,239 @@
+import { DialogManager, Dialog, EventRoot } from './DialogManager';
+
+describe('DialogManager', () => {
+    describe('#register', () => {
+        it('adds the `dialogManager.handleKeydown` event listener to the given event root if invoked for the first time', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = new DialogManager({ eventRoot });
+            const dialog: Dialog = { close: jest.fn() };
+
+            dialogManager.register(dialog);
+
+            expect(eventRoot.addEventListener).toBeCalledWith(
+                'keydown',
+                dialogManager.handleKeydown
+            );
+        });
+
+        it('does not add the event listener to the given event root on subsequent calls', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = new DialogManager({ eventRoot });
+            const dialog1: Dialog = { close: jest.fn() };
+            const dialog2: Dialog = { close: jest.fn() };
+            const dialog3: Dialog = { close: jest.fn() };
+
+            dialogManager.register(dialog1);
+            dialogManager.register(dialog2);
+            dialogManager.register(dialog3);
+
+            expect(eventRoot.addEventListener).toBeCalledTimes(1);
+        });
+
+        it('re-adds the event listener to the given event root if invoked after all dialogs have been closed', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = new DialogManager({ eventRoot });
+            const dialog1: Dialog = { close: jest.fn() };
+            const dialog2: Dialog = { close: jest.fn() };
+            const dialog3: Dialog = { close: jest.fn() };
+
+            // Register dialogs
+            dialogManager.register(dialog1);
+            dialogManager.register(dialog2);
+            dialogManager.register(dialog3);
+
+            // Close all dialogs
+            dialogManager.closeLatest();
+            dialogManager.closeLatest();
+            dialogManager.closeLatest();
+
+            expect(eventRoot.addEventListener).toBeCalledTimes(1);
+
+            // Register another dialog
+            dialogManager.register(dialog1);
+
+            expect(eventRoot.addEventListener).toBeCalledTimes(2);
+        });
+    });
+
+    describe('#handleKeydown', () => {
+        it('invokes `dialogManager.closeLatest` if the given event was an Escape-Keypress', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = Object.assign(
+                new DialogManager({ eventRoot }),
+                { closeLatest: jest.fn() }
+            );
+            const event: KeyboardEvent = {
+                key: 'Escape',
+            } as KeyboardEvent;
+
+            dialogManager.handleKeydown(event);
+
+            expect(dialogManager.closeLatest).toBeCalled();
+        });
+        it('does not invoke `dialogManager.closeLatest` if the given event was not an Escape-Keypress', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = Object.assign(
+                new DialogManager({ eventRoot }),
+                { closeLatest: jest.fn() }
+            );
+
+            dialogManager.handleKeydown({
+                key: 'A',
+            } as KeyboardEvent);
+
+            expect(dialogManager.closeLatest).not.toBeCalled();
+
+            dialogManager.handleKeydown({
+                key: 'Foo',
+            } as KeyboardEvent);
+
+            expect(dialogManager.closeLatest).not.toBeCalled();
+        });
+    });
+
+    describe('#closeLatest', () => {
+        it('picks the latest registered dialog and invokes `dialog.close` on it', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = new DialogManager({ eventRoot });
+            const dialog1: Dialog = { close: jest.fn() };
+            const dialog2: Dialog = { close: jest.fn() };
+            const dialog3: Dialog = { close: jest.fn() };
+
+            dialogManager.register(dialog1);
+            dialogManager.register(dialog2);
+            dialogManager.register(dialog3);
+
+            dialogManager.closeLatest();
+            expect(dialog1.close).not.toHaveBeenCalled();
+            expect(dialog2.close).not.toHaveBeenCalled();
+            expect(dialog3.close).toHaveBeenCalled();
+
+            dialogManager.closeLatest();
+            expect(dialog1.close).not.toHaveBeenCalled();
+            expect(dialog2.close).toHaveBeenCalled();
+            expect(dialog3.close).toHaveBeenCalledTimes(1);
+
+            dialogManager.closeLatest();
+            expect(dialog1.close).toHaveBeenCalled();
+            expect(dialog2.close).toHaveBeenCalledTimes(1);
+            expect(dialog3.close).toHaveBeenCalledTimes(1);
+
+            dialogManager.closeLatest();
+            expect(dialog1.close).toHaveBeenCalledTimes(1);
+            expect(dialog2.close).toHaveBeenCalledTimes(1);
+            expect(dialog3.close).toHaveBeenCalledTimes(1);
+        });
+
+        it('removes the `dialogManager.handleKeydown` event listener from the given event root once all dialogs have been closed', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = new DialogManager({ eventRoot });
+            const dialog1: Dialog = { close: jest.fn() };
+            const dialog2: Dialog = { close: jest.fn() };
+            const dialog3: Dialog = { close: jest.fn() };
+
+            dialogManager.register(dialog1);
+            dialogManager.register(dialog2);
+            dialogManager.register(dialog3);
+
+            dialogManager.closeLatest();
+            dialogManager.closeLatest();
+
+            expect(eventRoot.removeEventListener).not.toBeCalled();
+
+            dialogManager.closeLatest();
+
+            expect(eventRoot.removeEventListener).toBeCalledWith(
+                'keydown',
+                dialogManager.handleKeydown
+            );
+
+            dialogManager.closeLatest();
+
+            expect(eventRoot.removeEventListener).toBeCalledTimes(1);
+        });
+
+        it('closes a registered dialog only once, even if has been registered twice - in which case it uses order of first registration', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = new DialogManager({ eventRoot });
+            const dialog1: Dialog = { close: jest.fn() };
+            const dialog2: Dialog = { close: jest.fn() };
+
+            dialogManager.register(dialog1);
+            dialogManager.register(dialog2);
+
+            // Register dialog 1 again
+            dialogManager.register(dialog1);
+
+            dialogManager.closeLatest();
+            expect(dialog1.close).not.toBeCalled();
+
+            dialogManager.closeLatest();
+            expect(dialog1.close).toBeCalled();
+        });
+    });
+
+    describe('#forget', () => {
+        it('removes a dialog from the stack', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = new DialogManager({ eventRoot });
+            const dialog1: Dialog = { close: jest.fn() };
+            const dialog2: Dialog = { close: jest.fn() };
+            const dialog3: Dialog = { close: jest.fn() };
+
+            dialogManager.register(dialog1);
+            dialogManager.register(dialog2);
+            dialogManager.register(dialog3);
+
+            dialogManager.forget(dialog2);
+
+            dialogManager.closeLatest();
+            dialogManager.closeLatest();
+            dialogManager.closeLatest();
+
+            expect(dialog1.close).toHaveBeenCalled();
+            expect(dialog2.close).not.toHaveBeenCalled();
+            expect(dialog3.close).toHaveBeenCalled();
+        });
+
+        it('removes the `dialogManager.handleKeydown` event listener from the given event root if the last remaining dialog is removed', () => {
+            const eventRoot: EventRoot = {
+                addEventListener: jest.fn(),
+                removeEventListener: jest.fn(),
+            };
+            const dialogManager = new DialogManager({ eventRoot });
+            const dialog: Dialog = { close: jest.fn() };
+
+            dialogManager.register(dialog);
+            dialogManager.forget(dialog);
+
+            expect(eventRoot.removeEventListener).toHaveBeenCalled();
+        });
+    });
+});
diff --git a/packages/react-ui-components/src/Dialog/DialogManager.ts b/packages/react-ui-components/src/Dialog/DialogManager.ts
@@ -0,0 +1,52 @@
+export interface EventRoot {
+    addEventListener: Document['addEventListener'];
+    removeEventListener: Document['removeEventListener'];
+}
+
+export interface Dialog {
+    close: () => void;
+}
+
+export class DialogManager {
+    private dialogs: Dialog[] = [];
+
+    constructor(private readonly deps: { eventRoot: EventRoot }) {}
+
+    public register(dialog: Dialog): void {
+        if (this.dialogs.length === 0) {
+            this.deps.eventRoot.addEventListener('keydown', this.handleKeydown);
+        }
+
+        if (!this.dialogs.includes(dialog)) {
+            this.dialogs.push(dialog);
+        }
+    }
+
+    public forget(dialog: Dialog): void {
+        this.dialogs = this.dialogs.filter((d) => d !== dialog);
+        this.removeHandleKeydownEventListenerIfNecessary();
+    }
+
+    public readonly handleKeydown = (event: KeyboardEvent): void => {
+        if (event.key === 'Escape') {
+            this.closeLatest();
+        }
+    }
+
+    public closeLatest(): void {
+        const dialog = this.dialogs.pop();
+        if (dialog) {
+            dialog.close();
+            this.removeHandleKeydownEventListenerIfNecessary();
+        }
+    }
+
+    private removeHandleKeydownEventListenerIfNecessary(): void {
+        if (this.dialogs.length === 0) {
+            this.deps.eventRoot.removeEventListener(
+                'keydown',
+                this.handleKeydown
+            );
+        }
+    }
+}