Skip to content
This repository has been archived by the owner on Dec 20, 2024. It is now read-only.

Vinicius: Contra Frontend Assessment #94

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

vi-hidden-alt
Copy link

Hello there :)

This is my submission, featuring:

  • infinitely nested modals
  • correct focus-trapping behavior
  • scroll locking
  • custom modal mount point
  • automated tests

Went a little bit over time because I had to spend some time editing my Looms. Whoops.

Loom with a demo of the modal:
https://www.loom.com/share/6ef1439a4f5340798870207ff81a64a5

Loom with some of the code:
https://www.loom.com/share/d9c710a8f5f040b9a69fd2584d3d3671

Comment on lines +78 to +92
it('closes on clicking outside the modal', async () => {
expect.assertions(3);

render(<ToggleableModal />);

expect(screen.queryByText('Modal content')).not.toBeInTheDocument();

await userEvent.click(screen.getByText('Toggle modal'));

expect(screen.getByText('Modal content')).toBeInTheDocument();

await userEvent.click(document.body);

expect(screen.getByText('Modal content')).toBeInTheDocument();
});
Copy link
Author

@vi-hidden-alt vi-hidden-alt Apr 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last assertion in this test is incorrect - it's inverted. Looking back I assume the correct way wouldn't pass because I accidentally used document.addEventListener instead of document.body.addEventListener - and I'm clicking directly on document.body in this test. Either way, I was out of time at the moment I realized it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant