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

Add Modal and ModalDialog Tests #1121

Merged
merged 6 commits into from
Apr 2, 2020
Merged

Add Modal and ModalDialog Tests #1121

merged 6 commits into from
Apr 2, 2020

Conversation

thayannevls
Copy link
Contributor

@thayannevls thayannevls commented Mar 25, 2020

What is the purpose of this pull request?

#trivial: Add Modal and ModalDialog tests.

Obs: Since Modal uses Portal, I had to change the container in all tests to work properly.

What problem is this solving?

#1083

How should this be manually tested?

yarn && yarn test

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Requires change to documentation, which has been updated accordingly.

@thayannevls thayannevls changed the base branch from master to features March 25, 2020 19:15
@thayannevls thayannevls force-pushed the chore/modal-tests branch 4 times, most recently from 8b0c3af to dd120af Compare March 26, 2020 16:53
@thayannevls thayannevls marked this pull request as ready for review March 26, 2020 16:57
@thayannevls thayannevls requested a review from a team March 26, 2020 16:57
@thayannevls thayannevls requested a review from a team as a code owner March 26, 2020 16:57
react/components/Modal/index.test.js Outdated Show resolved Hide resolved
@thayannevls thayannevls requested a review from kevinch March 26, 2020 18:02
kevinch
kevinch previously approved these changes Mar 26, 2020
>
Foo
</ModalDialog>,
{ container: document.body }
Copy link
Contributor Author

@thayannevls thayannevls Mar 26, 2020

Choose a reason for hiding this comment

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

I couldn't remove thedocument.body here because without it I can't find the button by the text, but I believe that when I rewrite the component I'll be able to capture by using test-id or something similar

@thayannevls thayannevls requested a review from kevinch March 30, 2020 18:18
kevinch
kevinch previously approved these changes Mar 31, 2020
estacioneto
estacioneto previously approved these changes Mar 31, 2020
Copy link
Member

@emersonlaurentino emersonlaurentino left a comment

Choose a reason for hiding this comment

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

Great Job @thayannevls

@thayannevls thayannevls self-assigned this Apr 2, 2020
@matheusps matheusps merged commit 3bb2927 into features Apr 2, 2020
@matheusps matheusps deleted the chore/modal-tests branch April 2, 2020 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants