-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Solution #1592
base: master
Are you sure you want to change the base?
Solution #1592
Conversation
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.
Almost perfect!
src/components/Person/Person.jsx
Outdated
const { name, | ||
age, | ||
sex, | ||
isMarried, | ||
partnerName } = person; |
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.
const { name, | |
age, | |
sex, | |
isMarried, | |
partnerName } = person; | |
const { | |
name, | |
age, | |
sex, | |
isMarried, | |
partnerName, | |
} = person; |
src/components/Person/Person.jsx
Outdated
sex, | ||
isMarried, | ||
partnerName } = person; | ||
const partnerDesignation = sex === 'm' |
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.
const partnerDesignation = sex === 'm' | |
const partner = sex === 'm' |
You can simplify the variable's name
src/components/Person/Person.jsx
Outdated
return ( | ||
<section className="Person"> | ||
<h2 className="Person__name">{`My name is ${name}`}</h2> | ||
{person.age && (<p className="Person__age">{`I am ${age}`}</p>)} |
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.
It's not critical, but you could check if the age is a positive number
src/components/Person/Person.jsx
Outdated
<h2 className="Person__name">{`My name is ${name}`}</h2> | ||
{person.age && (<p className="Person__age">{`I am ${age}`}</p>)} |
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.
<h2 className="Person__name">{`My name is ${name}`}</h2> | |
{person.age && (<p className="Person__age">{`I am ${age}`}</p>)} | |
<h2 className="Person__name"> | |
{`My name is ${name}`} | |
</h2> | |
{person.age && ( | |
<p className="Person__age"> | |
{`I am ${age}`} | |
</p> | |
)} |
Such formatting will make your code easier to read
{`My name is ${name}`} | ||
</h2> | ||
|
||
{person.age && ( |
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.
why use person.age here? just use age ;)
btw, I guess if age is gonna be 0 it's not gonna work correctly, I mean age 0 is not correctly itself, but react will probably just render '0' on the page, so we have to convert it to boolean to make sure it works just fine, we can achieve it by using !! or Boolean
DEMO LINK