Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX: Limit click-outside handling for dialogs to their semi-transparent overlay #3492

Merged
merged 6 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion packages/react-ui-components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
239 changes: 239 additions & 0 deletions packages/react-ui-components/src/Dialog/DialogManager.spec.ts
Original file line number Diff line number Diff line change
@@ -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();
mhsdesign marked this conversation as resolved.
Show resolved Hide resolved

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();
});
});
});
52 changes: 52 additions & 0 deletions packages/react-ui-components/src/Dialog/DialogManager.ts
Original file line number Diff line number Diff line change
@@ -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();
}
mhsdesign marked this conversation as resolved.
Show resolved Hide resolved
}

private removeHandleKeydownEventListenerIfNecessary(): void {
if (this.dialogs.length === 0) {
this.deps.eventRoot.removeEventListener(
'keydown',
this.handleKeydown
);
}
}
}
Loading
Loading