Skip to content

Commit

Permalink
fix(CustomModal): do not prevent default behavior of click inside modal
Browse files Browse the repository at this point in the history
* fix(CustomModal): do not prevent default behavior of click inside modal

By blinldy preventing default behavior on click, basic html elements
like the anchor, in which the click is left to be the default one,
stop working.

Remove the prevention of the default behaviour from the custom modal,
and leave the external user in charge of preventing what needs to be prevented.

* test: add jest-fail-on-console to make test fail on console errors
* test(Modal, CustomModal): test default behaviour for anchor tag
* refactor(Modal,CustomModal): merge implementation of modals and remove timers

Since the implementation of the modal is the same, use a CustomModal also to render
a standard Modal, in order to remove the duplicated code and prevent differences
in the two components.

Remove the timers where possible

Use the containerWindow body of the container window if provided

refs: CDS-102 (#218)
  • Loading branch information
beawar authored Sep 20, 2023
1 parent d51c42a commit 33a8952
Show file tree
Hide file tree
Showing 9 changed files with 309 additions and 295 deletions.
140 changes: 140 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"husky": "^8.0.3",
"is-ci": "3.0.1",
"jest": "26.6.3",
"jest-fail-on-console": "^3.1.1",
"jest-junit": "13.0.0",
"jest-styled-components": "7.1.1",
"lodash": "^4.17.21",
Expand Down
6 changes: 6 additions & 0 deletions src/components/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,9 @@
*/
export const INPUT_BACKGROUND_COLOR = 'gray5';
export const INPUT_DIVIDER_COLOR = 'gray3';

export const TIMERS = {
MODAL: {
DELAY_OPEN: 1
}
};
65 changes: 35 additions & 30 deletions src/components/feedback/CustomModal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { render } from '../../test-utils';
import { Button } from '../basic/Button';
import { Text } from '../basic/Text';

const ModalTester = (props: CustomModalProps): React.JSX.Element => {
const ModalTester = ({ children, ...props }: CustomModalProps): React.JSX.Element => {
const [open, setOpen] = useState(false);
const clickHandler = (): void => setOpen(true);
const closeHandler = (): void => setOpen(false);
Expand All @@ -23,15 +23,19 @@ const ModalTester = (props: CustomModalProps): React.JSX.Element => {
<>
<Button label="Trigger Modal" onClick={clickHandler} />
<CustomModal {...props} open={open} onClose={closeHandler}>
<Text>My Title</Text>
<Text overflow="break-word">Lorem ipsum dolor sit amet.</Text>
{children || (
<>
<Text>My Title</Text>
<Text overflow="break-word">Lorem ipsum dolor sit amet.</Text>
</>
)}
</CustomModal>
</>
);
};

describe('Custom Modal', () => {
test('Render Modal', () => {
test('Render Modal', async () => {
render(<ModalTester />);

const button = screen.getByRole('button', { name: /trigger modal/i });
Expand All @@ -40,9 +44,8 @@ describe('Custom Modal', () => {
expect(screen.queryByText('Lorem ipsum dolor sit amet.')).not.toBeInTheDocument();

userEvent.click(button);

expect(screen.getByText('My Title')).toBeInTheDocument();
expect(screen.getByText('Lorem ipsum dolor sit amet.')).toBeInTheDocument();
await waitFor(() => expect(screen.getByText('My Title')).toBeVisible());
expect(screen.getByText('Lorem ipsum dolor sit amet.')).toBeVisible();
expect(button).toBeInTheDocument();
});

Expand All @@ -51,26 +54,13 @@ describe('Custom Modal', () => {
render(<ModalTester onClick={onClick} />);

const button = screen.getByRole('button', { name: /trigger modal/i });
expect(button).toBeInTheDocument();
expect(screen.queryByText('My Title')).not.toBeInTheDocument();
expect(screen.queryByText('Lorem ipsum dolor sit amet.')).not.toBeInTheDocument();

userEvent.click(button);

// wait for listeners to be registered
await waitFor(
() =>
new Promise((resolve) => {
setTimeout(resolve, 1);
})
);
expect(screen.getByText('My Title')).toBeVisible();
expect(screen.getByText('Lorem ipsum dolor sit amet.')).toBeVisible();
await waitFor(() => expect(screen.getByText('My Title')).toBeVisible());
const overlayElement = screen.getByTestId('modal');
expect(overlayElement).toBeVisible();
userEvent.click(overlayElement);
expect(screen.queryByText('My Title')).not.toBeInTheDocument();
expect(screen.queryByText('Lorem ipsum dolor sit amet.')).not.toBeInTheDocument();
expect(onClick).not.toHaveBeenCalled();
});

Expand All @@ -81,22 +71,37 @@ describe('Custom Modal', () => {
const button = screen.getByRole('button', { name: /trigger modal/i });
expect(button).toBeInTheDocument();
expect(screen.queryByText('My Title')).not.toBeInTheDocument();
expect(screen.queryByText('Lorem ipsum dolor sit amet.')).not.toBeInTheDocument();

userEvent.click(button);
await waitFor(() => expect(screen.getByText('My Title')).toBeVisible());
userEvent.click(screen.getByText('My Title'));
expect(screen.getByText('My Title')).toBeVisible();
expect(onClick).toHaveBeenCalled();
});

// wait for listeners to be registered
test('should not blindly prevent default behavior of html elements', async () => {
const originalConsoleError = console.error;
const errors: string[] = [];
console.error = (message): void => {
errors.push(message);
};
const href = '/different-path';
render(
<ModalTester>
<a href={href}>This is a link</a>
</ModalTester>
);
userEvent.click(screen.getByRole('button'));
userEvent.click(screen.getByRole('link'));
await waitFor(
() =>
new Promise((resolve) => {
// wait for the navigation callback of the jsdom hyperlink implementation to be called
setTimeout(resolve, 1);
})
);
expect(screen.getByText('My Title')).toBeVisible();
expect(screen.getByText('Lorem ipsum dolor sit amet.')).toBeVisible();
userEvent.click(screen.getByText('My Title'));
expect(screen.getByText('My Title')).toBeVisible();
expect(screen.getByText('Lorem ipsum dolor sit amet.')).toBeVisible();
expect(onClick).toHaveBeenCalled();
expect(errors).toEqual([
expect.stringContaining('Error: Not implemented: navigation (except hash changes)')
]);
console.error = originalConsoleError;
});
});
Loading

0 comments on commit 33a8952

Please sign in to comment.