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

conditional rendering page, done translation till line 258 #176

Merged

Conversation

sanchit36
Copy link
Contributor

@sanchit36 sanchit36 commented Jan 15, 2022

Translating the conditional rendering page.
Related to #168
Review Progress

  • Intro
  • Conditionally returning JSX
    • Conditionally returning nothing with null
  • Conditionally including JSX
    • Conditional (ternary) operator (? :)
    • Logical AND operator (&&)
    • Conditionally assigning JSX to a variable
    • Show an icon for incomplete items with ? :
    • Show the item importance with &&
    • Refactor a series of ? : to if and variables

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 work.

I have added comments till line 106

Lets get these comments fixed, post that we can start reviewing rest of the file.

Also you can refer #23 for our review process

beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
@arshadkazmi42 arshadkazmi42 added the 1st Review First phase of review label Jan 15, 2022
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.

Awesome work with the previous fixes and thanks alot for following the review guidelines.

I have added review for rest of the file.

Once these are fixed, we can get this merged.

beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
@@ -254,7 +255,7 @@ export default function PackingList() {

</Sandpack>

This style works well for simple conditions, but use it in moderation. If your components get messy with too much nested conditional markup, consider extracting child components to clean things up. In React, markup is a part of your code, so you can use tools like variables and functions to tidy up complex expressions.
यह शैली साधारण परिस्थितियों के लिए अच्छी तरह से काम करती है, लेकिन इसे मॉडरेशन में उपयोग करें। यदि आपके कौम्पोनॅन्ट बहुत अधिक नेस्टेड कंडीशनल मार्कअप के साथ गड़बड़ हो जाते हैं, तो चीजों को साफ करने के लिए बाल कौम्पोनॅन्टस को निकालने पर विचार करें। React में, मार्कअप आपके कोड का एक हिस्सा है, इसलिए आप जटिल अभिव्यक्तियों को व्यवस्थित करने के लिए वेरिएबल और फ़ंक्शन जैसे टूल का उपयोग कर सकते हैं।
Copy link
Member

Choose a reason for hiding this comment

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

शैली => तरीका

beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
beta/src/pages/learn/conditional-rendering.md Outdated Show resolved Hide resolved
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.

Awesome work.

I have verified the full file.

Lets get this merged 🎉

@arshadkazmi42 arshadkazmi42 merged commit 8a8eb26 into reactjs:main Jan 15, 2022
@sanchit36
Copy link
Contributor Author

Thank you, this was my first open source contribution 😃.

@arshadkazmi42
Copy link
Member

Congrats @sanchit36 for landing your first PR.

You did a great job at this PR. Quality of PR was great.

Best of luck of your future contributions 👍

@sanchit36
Copy link
Contributor Author

@arshadkazmi42 the page is not completed yet, Conditionally including JSX i have to complete it's subparts.

@arshadkazmi42
Copy link
Member

Oh. I thought its completed. feel free to send a new PR.

@sanchit36
Copy link
Contributor Author

Should i send a new one or continue in this one only as its the same page?

@arshadkazmi42
Copy link
Member

@sanchit36 I have already merged the PR. You can make changes in same branch. But you will have to create a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1st Review First phase of review beta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants