Skip to content

Conversation

lbelinsk
Copy link
Contributor

@lbelinsk lbelinsk commented May 31, 2018

Fixes #580 .
Related to #576.

Changes proposed:

  • Added missing uncovered test for the scenario when a user wants to use `ReactModal.setElement("some_element"), i.e. on a string type appElement.
  • Added some safety check in settlement method when there is no document due to ssr for example. It actually broke my project which uses ssr. I don't see any tests for ssr, but if for some reasons I missed them, I will be more than happy to add a test for my change.

Upgrade Path (for changed or removed APIs):

Acceptance Checklist:

  • The commit message follows the guidelines in CONTRIBUTING.md.
  • Documentation (README.md) and examples have been updated as needed. - Not needed.
  • If this is a code change, a spec testing the functionality has been added.
  • If the commit message has [changed] or [removed], there is an upgrade path above. - Not relevant

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 87.703% when pulling 3f1ec6d on lbelinsk:master into 5f92df7 on reactjs:master.

@diasbruno
Copy link
Collaborator

@lbelinsk Thank you for having a look on this.

@diasbruno diasbruno merged commit 73893a2 into reactjs:master Jun 1, 2018
@diasbruno
Copy link
Collaborator

Released v3.4.5.

@lbelinsk
Copy link
Contributor Author

lbelinsk commented Jun 2, 2018

No problem!
Thanks for the quick merge :)

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.

3 participants