Uh oh!
There was an error while loading. Please reload this page.
fix bug from issue #460 - #464
Conversation
dimensi
commented
Mar 5, 2019
@jquense ? |
silvenon
commented
Mar 5, 2019
Reviewing your PR now. 👀 |
There was a problem hiding this comment.
Could you give more details about what your tests are proving? By "at once" do you mean "immediately"? How is time measured here?
I was thinking that actually 0 does make sense as a default timeout value for enter and exit. I think this would be the most logical step forward and wouldn't cause warnings for existing code. What do you think?
If you agree, would you mind reverting the change for prop types and instead of current tests add simpler tests where by measuring time you ensure that:
- in the absence of
enter, the status upon entering immediately becomesentered - in the absence of
exit, the status upon exiting immediately becomesexited
I don't think we have to test the unmountOnExit prop for this, we just want to ensure the default values.
Concerning prop types, you can take this opportunity to actually fix them: PropTypes.oneOfType([PropTypes.number,PropTypes.shape({enter: PropTypes.number,exit: PropTypes.number,appear: PropTypes.number,})]).isRequiredBy moving I'm honestly not sure what |
silvenon
left a comment
There was a problem hiding this comment.
Here is the summary of requested changes. Sorry for so many comments, it's a bit confusing because changes themselves are overlapping. 😅
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-Authored-By: dimensi <eddimensi@gmail.com>
dimensi
commented
Mar 5, 2019
yes, i don't know english, i use google translate for find a word. |
silvenon
commented
Mar 5, 2019
Awesome, thank you! 🏆 |
dimensi
commented
Mar 5, 2019
@silvenon thx for your review. Are you sure my tests right writed? |
silvenon
commented
Mar 5, 2019
@dimensi yep, they look good to me. I didn't even think to combine both functionalities in the same test. We might later split that into two tests because they are two different features. |
jquense
commented
Mar 14, 2019
🎉 This PR is included in version 2.6.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
I found an #460 with a bug, since the bug is simple, I decided to correct the error.
The error was that we did not take into account the case when there is a node and there is no timeout.
I also set the enter and exit values in prop-types as reqiured.