Skip to content

Commit

Permalink
BUGFIX: 3531 Prevent closing of node creation dialog
Browse files Browse the repository at this point in the history
  • Loading branch information
mhsdesign committed Jul 11, 2023
1 parent bd17979 commit 6d8d7a6
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ export default class NodeCreationDialog extends PureComponent {
actions={[this.renderBackAction(), this.renderSaveAction()]}
title={this.renderTitle()}
onRequestClose={this.handleCancel}
preventClosing={true}
type="success"
isOpen
style={this.state.secondaryInspectorComponent ? 'jumbo' : 'wide'}
Expand Down
66 changes: 48 additions & 18 deletions packages/react-ui-components/src/Dialog/DialogManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ describe('DialogManager', () => {
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() };
const dialog1: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog2: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog3: Dialog = { close: jest.fn().mockReturnValue(true) };

dialogManager.register(dialog1);
dialogManager.register(dialog2);
Expand All @@ -41,9 +41,9 @@ describe('DialogManager', () => {
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() };
const dialog1: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog2: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog3: Dialog = { close: jest.fn().mockReturnValue(true) };

// Register dialogs
dialogManager.register(dialog1);
Expand Down Expand Up @@ -113,9 +113,9 @@ describe('DialogManager', () => {
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() };
const dialog1: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog2: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog3: Dialog = { close: jest.fn().mockReturnValue(true) };

dialogManager.register(dialog1);
dialogManager.register(dialog2);
Expand Down Expand Up @@ -148,9 +148,9 @@ describe('DialogManager', () => {
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() };
const dialog1: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog2: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog3: Dialog = { close: jest.fn().mockReturnValue(true) };

dialogManager.register(dialog1);
dialogManager.register(dialog2);
Expand All @@ -173,14 +173,44 @@ describe('DialogManager', () => {
expect(eventRoot.removeEventListener).toBeCalledTimes(1);
});

it('but `dialog.close` prevents a close by returning `false`', () => {
const eventRoot: EventRoot = {
addEventListener: jest.fn(),
removeEventListener: jest.fn(),
};
const dialogManager = new DialogManager({ eventRoot });

const closeMock = jest.fn();

const dialog1: Dialog = { close: closeMock };

// prevent close
closeMock.mockReturnValue(false);

dialogManager.register(dialog1);

dialogManager.closeLatest();
expect(dialog1.close).toHaveBeenCalledTimes(1);

expect(eventRoot.removeEventListener).not.toBeCalled();

closeMock.mockReturnValue(true);

// allow close
dialogManager.closeLatest();
expect(dialog1.close).toHaveBeenCalledTimes(2);

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() };
const dialog1: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog2: Dialog = { close: jest.fn().mockReturnValue(true) };

dialogManager.register(dialog1);
dialogManager.register(dialog2);
Expand All @@ -203,9 +233,9 @@ describe('DialogManager', () => {
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() };
const dialog1: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog2: Dialog = { close: jest.fn().mockReturnValue(true) };
const dialog3: Dialog = { close: jest.fn().mockReturnValue(true) };

dialogManager.register(dialog1);
dialogManager.register(dialog2);
Expand All @@ -228,7 +258,7 @@ describe('DialogManager', () => {
removeEventListener: jest.fn(),
};
const dialogManager = new DialogManager({ eventRoot });
const dialog: Dialog = { close: jest.fn() };
const dialog: Dialog = { close: jest.fn().mockReturnValue(true) };

dialogManager.register(dialog);
dialogManager.forget(dialog);
Expand Down
9 changes: 6 additions & 3 deletions packages/react-ui-components/src/Dialog/DialogManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export interface EventRoot {
}

export interface Dialog {
close: () => void;
close: () => boolean;
}

export class DialogManager {
Expand Down Expand Up @@ -36,8 +36,11 @@ export class DialogManager {
public closeLatest(): void {
const dialog = this.dialogs.pop();
if (dialog) {
dialog.close();
this.removeHandleKeydownEventListenerIfNecessary();
if (dialog.close()) {
this.removeHandleKeydownEventListenerIfNecessary();
} else {
this.dialogs.push(dialog);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/react-ui-components/src/Dialog/dialog.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('<Dialog/>', () => {
'dialog__contents': 'contentsClassName',
'dialog__contentsPosition': 'contentsPositionClassName',
'dialog__title': 'titleClassName',
'dialog--effect__shake': 'effectShakeClassName'
},
title: 'Foo title',
};
Expand Down
42 changes: 39 additions & 3 deletions packages/react-ui-components/src/Dialog/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ interface DialogTheme {
readonly 'dialog--success': string;
readonly 'dialog--warn': string;
readonly 'dialog--error': string;
readonly 'dialog--effect__shake': string;
}

export interface DialogProps {
Expand All @@ -33,6 +34,11 @@ export interface DialogProps {
*/
readonly onRequestClose: () => void;

/**
* An optional boolean flag to keep the user in the dialog.
*/
readonly preventClosing?: boolean;

/**
* The title to be rendered on top of the Dialogs contents.
*/
Expand Down Expand Up @@ -88,9 +94,36 @@ class DialogWithOverlay extends PureComponent<DialogProps> {
private ref?: HTMLDivElement;

private dialog: Dialog = {
close: this.props.onRequestClose,
close: () => {
if (this.props.preventClosing) {
this.startShaking();
return false;
}
this.props.onRequestClose();
return true;
},
};

public state: Readonly<{
isShaking: boolean
}> = {
isShaking: false
};

private startShaking = () => {
if (this.state.isShaking) {
return;
}
this.setState({
isShaking: true
});
setTimeout(() => {
this.setState({
isShaking: false
});
}, 820);
}

public renderDialogWithoutOverlay(): JSX.Element {
const { title, children, actions, theme, type } = this.props;

Expand All @@ -110,7 +143,10 @@ class DialogWithOverlay extends PureComponent<DialogProps> {
className={theme.dialog__contentsPosition}
tabIndex={0}
>
<div className={theme.dialog__contents}>
<div className={mergeClassNames(
theme.dialog__contents,
this.state.isShaking && theme['dialog--effect__shake']
)}>
<div className={theme.dialog__title}>{title}</div>
<div className={finalClassNameBody}>{children}</div>

Expand Down Expand Up @@ -193,7 +229,7 @@ class DialogWithOverlay extends PureComponent<DialogProps> {

private readonly handleOverlayClick = (ev: React.MouseEvent) => {
if (ev.target === ev.currentTarget) {
this.props.onRequestClose();
this.dialog.close();
}
}
}
Expand Down
30 changes: 30 additions & 0 deletions packages/react-ui-components/src/Dialog/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,33 @@
height: 100%;
width: 100%;
}

.dialog--effect__shake {
animation: shake .82s cubic-bezier(.36, .07, .19, .97) both;
transform: translate3d(0, 0, 0);
backface-visibility: hidden;
perspective: 1000px;
}
@keyframes shake {

10%,
90% {
transform: translate3d(-1px, 0, 0);
}

20%,
80% {
transform: translate3d(2px, 0, 0);
}

30%,
50%,
70% {
transform: translate3d(-4px, 0, 0);
}

40%,
60% {
transform: translate3d(4px, 0, 0);
}
}

0 comments on commit 6d8d7a6

Please sign in to comment.