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

Update conditional-rendering.md #36

Closed
wants to merge 17 commits into from
Closed

Update conditional-rendering.md #36

wants to merge 17 commits into from

Conversation

kambleaa007
Copy link

No description provided.

@netlify
Copy link

netlify bot commented Jun 11, 2019

Deploy preview for hi-reactjs ready!

Built with commit 8425d90

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

@vermasachin
Copy link
Contributor

I don't think you need to translate the word 'React'. Refer to #2 .

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.

Reviewed sucess

@arshadkazmi42
Copy link
Member

@kambleaa007 Let me know once you are done with the translation. I will start reviewing after that

@kambleaa007
Copy link
Author

@kambleaa007 Let me know once you are done with the translation. I will start reviewing after that

Hi, is this two lines are fine ?
I am now using Hindi Typing.
Thanks

@arshadkazmi42
Copy link
Member

Yes it looks fine. I will a deep check during review, you should follow #2 to check the words we are not translating and the hindi spelling of some complex words

@kambleaa007
Copy link
Author

Task Completed,
Please review

@arshadkazmi42
Copy link
Member

@kambleaa007 I see you have open another PR #39 which one do you want to get reviewed.
We can close the duplicate one

@kambleaa007
Copy link
Author

Yeh, i have closed duplicate one

@kambleaa007
Copy link
Author

till then, You can assign me other simple task :)

@kambleaa007
Copy link
Author

@kambleaa007 I see you have open another PR #39 which one do you want to get reviewed.
We can close the duplicate one

Yes keep #36 closed duplicate #39

@arshadkazmi42
Copy link
Member

till then, You can assign me other simple task :)

You can pick another page from #1 but let's first close this. There will be some review feedbacks.

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.

Good start.
I have reviewed till line 122.
Once you are done with fixing those, I will continue reviewing left over lines.
Also read #23 to know the review process and how to resolve the review feedbacks

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
It will render either `<LoginButton />` or `<LogoutButton />` depending on its current state. It will also render a `<Greeting />` from the previous example:
इस एक्साम्पल में हम बना रहे हे [स्टैटिफुल कम्पोनेनेट ] (/docs/state-and-lifecycle.html#adding-local-state-to-a-class) जिसे बोलते हे `LoginControl`.

जो करंट स्टेट के हिसाब से `<LoginButton />` या `<LogoutButton />` को रेंडर करेगा वो `<Greeting />` को भी रेंडर करेगा जैसे पिछले एक्साम्पल में बताया था :
Copy link
Member

Choose a reason for hiding this comment

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

एक्साम्पल => उदहारण

change this everywhere

Copy link
Member

Choose a reason for hiding this comment

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

not fixed

Copy link
Member

Choose a reason for hiding this comment

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

एक्साम्पल => उदहारण

change this everywhere

no fixed

@kambleaa007
Copy link
Author

I have resolved, reviewed all changes,
Like to take hooks-intro.md next tasks, big one with lots of theory
assign me that.

@arshadkazmi42
Copy link
Member

I have resolved, reviewed all changes,
Like to take hooks-intro.md next tasks, big one with lots of theory
assign me that.

Nice. Assigned you the page at #1

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.

I see only two fixes, did you forgot to push some of the changes. None of the issues are fixed except 2-3.
Also, do not resolve the feedback, reviewer will mark them resolved after review. You can just mark 👍 emoji to feedback on which you have worked.

@kambleaa007
Copy link
Author

I modified all suggested changes :), I thought resolving does auto modifications, what is the status of other contributors 👍

@arshadkazmi42
Copy link
Member

We would want separate pull requests for different pages, can you create a new PR for hook-intro page?
Also revert the hook-intro page changes from this PR.

@arshadkazmi42
Copy link
Member

Some of the feedbacks are left, can you check the unresolved feedback again

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.

Reviewed

@arshadkazmi42
Copy link
Member

There are still few lefts. Go above to this comment Comment
and check all unresolved feedback.
Those are all pending

@kambleaa007
Copy link
Author

too much overhead so closing this

@kambleaa007 kambleaa007 closed this Jul 9, 2019
@arshadkazmi42
Copy link
Member

I don't think you should close the PR. Since you have already raised a new one let's review that.
But review process will be like this, and we can't keep on creating new PRs.

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