-
Notifications
You must be signed in to change notification settings - Fork 131
API Reference > Test Utilities #45
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
API Reference > Test Utilities #45
Conversation
paramaggarwal
commented
Aug 23, 2019
•
edited
Loading
edited
- Overview
- Reference
- act()
- mockComponent()
- isElement()
- isElementOfType()
- isDOMComponent()
- isCompositeComponent()
- isCompositeComponentWithType()
- findAllInRenderedTree()
- scryRenderedDOMComponentsWithClass()
- findRenderedDOMComponentWithClass()
- scryRenderedDOMComponentsWithTag()
- findRenderedDOMComponentWithTag()
- scryRenderedComponentsWithType()
- findRenderedComponentWithType()
- renderIntoDocument()
- Other Utilities
- Simulate
Deploy preview for hi-reactjs ready! Built with commit c40d883 |
Deploy preview for hi-reactjs ready! Built with commit 0eaa33c |
@paramaggarwal Good start. Let me know once you are done translating the whole file. I will start the review |
Awesome. I will start reviewing this later today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great start.
I have added some feedbacks till line 124.
For review process do read #23
Thanks for the excellent feedback. I have addressed everything raised till now. We can continue further review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work with the fixes.
I have reviewed the whole file and add couple of more feedbacks.
Once these are fixed it will be good to go from my end for second phase of review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome 💯
This is good to go from my end.
For next steps, there will be a second round of review done by @saranshkataria. Once that is done, we will get this merged
Comments addressed. Sorry for previously marking comments as "Resolved" - have not done so this time. |
@paramaggarwal this has become confusing a bit to check which ones were resolved and which ones were not. Marking everything as resolved and starting the review afresh. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added comments till line 220
@paramaggarwal there are quite a few pending points. Could you look into those as well before we proceed with the review? |
Ready. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added final set of changes needed. Then this will be good to 👍
Ready. |
LGTM, Thanks! Merging it in |
Thanks a lot @saranshkataria and @arshadkazmi42 👍 |