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.

Sign up for GitHub

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

Jump to bottom

Condition rendering fixed updated final #43

Closed
wants to merge 28 commits into from
Closed

Condition rendering fixed updated final #43

wants to merge 28 commits into from

Conversation

kambleaa007
Copy link

No description provided.

@netlify
Copy link

netlify bot commented Jul 9, 2019 •
edited
Loading

Deploy preview for hi-reactjs ready!

Built with commit 19132f8

https://deploy-preview-43--hi-reactjs.netlify.com

arshadkazmi42
arshadkazmi42 requested changes Jul 13, 2019
View reviewed changes
Copy link
Member

@arshadkazmi42 arshadkazmi42 left a comment

Choose a reason for hiding this comment

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

Reviewed till 124.
There are still few feedbacks are pending which were not fixed in the previous PR and I have added a few more feedbacks
Let's get those fixed then I will continue the review further

content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
@arshadkazmi42
Copy link
Member

@kambleaa007 There is two left from previous feedback. Lets get those fixed and then we can continue with rest of the review

arshadkazmi42
arshadkazmi42 requested changes Jul 15, 2019
View reviewed changes
Copy link
Member

@arshadkazmi42 arshadkazmi42 left a comment

Choose a reason for hiding this comment

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

Done till line 155.

content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
arshadkazmi42
arshadkazmi42 requested changes Jul 16, 2019
View reviewed changes
Copy link
Member

@arshadkazmi42 arshadkazmi42 left a comment

Choose a reason for hiding this comment

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

Done with review.
Once you fix all the feedbacks it will be good to go from my end for second review

content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
@arshadkazmi42
Copy link
Member

@kambleaa007 Great work.
Thank you for the all the patience and feedback fixes. There are two more left and it will be good to go from my end

@kambleaa007
Copy link
Author

Done with review.
Once you fix all the feedbacks it will be good to go from my end for second review

Who will be the second reviewer ?

arshadkazmi42
arshadkazmi42 approved these changes Jul 18, 2019
View reviewed changes
Copy link
Member

@arshadkazmi42 arshadkazmi42 left a comment

Choose a reason for hiding this comment

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

Great work 💯
Thank you for your patience and working on the feedbacks.
Review is done from my end.
For next steps @saranshkataria will be doing second review for this.

@kambleaa007
Copy link
Author

Great work 💯
Thank you for your patience and working on the feedbacks.
Review is done from my end.
For next steps @saranshkataria will be doing second review for this.

Thanks @arshadkazmi42 for guiding me!!

@saranshkataria
Copy link
Member

I'll start looking into this soon. Sorry for the delay!

@tesseralis tesseralis mentioned this pull request Oct 8, 2019
Closed
88 tasks
@kambleaa007
Copy link
Author

@saranshkataria I guess, Your approval is pending

saranshkataria
saranshkataria reviewed Oct 14, 2019
View reviewed changes
content/docs/conditional-rendering.md Outdated
@@ -44,13 +43,13 @@ ReactDOM.render(

[**Try it on CodePen**](https://codepen.io/gaearon/pen/ZpVxNq?editors=0011)
Copy link
Member

Choose a reason for hiding this comment

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

this needs to be translated too

Copy link
Author

Choose a reason for hiding this comment

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

CodePen में कोशिश करो

Copy link
Member

Choose a reason for hiding this comment

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

इसे CodePen पर आज़माएँ makes more sense

saranshkataria
saranshkataria reviewed Oct 14, 2019
View reviewed changes
content/docs/conditional-rendering.md Outdated

Consider these two new components representing Logout and Login buttons:
अब लॉगिन और लॉगआउट बटन्स नए कौम्पोनॅन्टस को देखो :
Copy link
Member

Choose a reason for hiding this comment

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

not the right translation

Copy link
Author

Choose a reason for hiding this comment

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

लॉगिन और लॉगआउट बटन्स के नए कौम्पोनॅन्टस पर विचार करें

Copy link
Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

लॉगआउट और लॉगिन बटन्स का प्रतिनिधित्व करने वाले इन दो नए कौम्पोनॅन्टस पर विचार करें:

@saranshkataria
Copy link
Member

added comments till line 70

kambleaa007
kambleaa007 commented Oct 24, 2019
View reviewed changes
Copy link
Author

@kambleaa007 kambleaa007 left a comment

Choose a reason for hiding this comment

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

Done reviewing...

content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated
@@ -44,13 +43,13 @@ ReactDOM.render(

[**Try it on CodePen**](https://codepen.io/gaearon/pen/ZpVxNq?editors=0011)
Copy link
Author

Choose a reason for hiding this comment

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

CodePen में कोशिश करो

content/docs/conditional-rendering.md Outdated Show resolved Hide resolved
content/docs/conditional-rendering.md Outdated

Consider these two new components representing Logout and Login buttons:
अब लॉगिन और लॉगआउट बटन्स नए कौम्पोनॅन्टस को देखो :
Copy link
Author

Choose a reason for hiding this comment

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

लॉगिन और लॉगआउट बटन्स के नए कौम्पोनॅन्टस पर विचार करें

content/docs/conditional-rendering.md Outdated

Consider these two new components representing Logout and Login buttons:
अब लॉगिन और लॉगआउट बटन्स नए कौम्पोनॅन्टस को देखो :
Copy link
Author

Choose a reason for hiding this comment

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

done

@saranshkataria
Copy link
Member

return

Welcome back!

; and return

Please sign up.

;
on lines18 and 22 should be translated to hindi too

@arshadkazmi42 arshadkazmi42 added the 2nd Review Second phrase of review label Jan 24, 2020
@gaearon gaearon deleted the branch reactjs:master August 5, 2021 17:46
@gaearon gaearon closed this Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arshadkazmi42 arshadkazmi42 arshadkazmi42 approved these changes

@saranshkataria saranshkataria saranshkataria left review comments

Assignees
No one assigned
Labels
2nd Review Second phrase of review awaiting author response
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kambleaa007 @arshadkazmi42 @saranshkataria @gaearon