solution#3795
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because your solution still violates explicit checklist items related to ternary formatting and conditional rendering, which are treated as required, not optional style preferences. Functionally, everything works correctly: the Person component displays the right data, the three people are rendered as required, and marriage/partner text behaves as specified. However, in src/App.jsx the ternaries for partnerSex and ageText are written on a single line; per [CHECKLIST ITEM #2], you need to split the condition, ? value, and : value onto separate lines for readability (e.g., const partnerSex = person.sex === 'm' ? 'wife' : 'husband';). Similarly, in src/components/Person/Person.jsx the partnerSex and ageText ternaries must be reformatted so the condition and each operand are on their own lines. In addition, the JSX {ageText && <p className="Person__age">{ageText}</p>} combines the condition and the rendered element inline, which goes against [CHECKLIST ITEM #3]; introduce a separate variable like const shouldShowAge = Boolean(ageText); and then use {shouldShowAge && ...} in the JSX. Once you address these checklist-related code-structure issues, your solution will fully meet both the functional and style requirements. The core logic is solid, so focus only on these formatting and conditional rendering adjustments, and you’ll be in great shape.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| @@ -1,5 +1,6 @@ | |||
| import React from 'react'; | |||
| import './App.scss'; | |||
There was a problem hiding this comment.
This violates checklist item #2: Readabily is everything. Format ternary operator operands correctly - move each operand to the separate line. Your ternary is currently on one line (const partnerSex = person.sex === 'm' ? 'wife' : 'husband';), which matches the Bad example pattern. Consider splitting the operands onto separate lines as shown in the Good example.
| import { Person } from './components/Person/Person'; | ||
|
|
||
| export const misha = { | ||
| name: 'Misha', |
There was a problem hiding this comment.
This line also uses a single-line ternary (const ageText = person.age ? \I am ${person.age}` : '';), which matches the **Bad example** from checklist item #2. Please reformat it so that the condition, firstValue, and secondValue` are each on their own line.
| @@ -1 +1,15 @@ | |||
| // export const Person = ({ person }) => (); | |||
| export const Person = ({ person }) => { | |||
| const partnerSex = person.sex === 'm' ? 'wife' : 'husband'; | |||
There was a problem hiding this comment.
This violates checklist item #2: "Format ternary operator operands correctly - move each operand to the separate line". The partnerSex ternary currently keeps the condition and operands on one line; update it so the condition, ? branch, and : branch are each on separate lines.
| const partnerText = person.isMarried | ||
| ? `${person.partnerName} is my ${partnerSex}` | ||
| : 'I am not married'; | ||
| const ageText = person.age ? `I am ${person.age}` : ''; |
There was a problem hiding this comment.
This also violates checklist item #2: "Format ternary operator operands correctly - move each operand to the separate line". The ageText ternary is written inline; reformat it so the condition, true value, and false value are each on their own lines.
| return ( | ||
| <div className="Person"> | ||
| <h2 className="Person__name">My name is {person.name}</h2> | ||
| {ageText && <p className="Person__age">{ageText}</p>} |
There was a problem hiding this comment.
This matches the "BAD EXAMPLE" pattern in checklist item #3: "Avoid putting several cases to conditional rendering. Create separate variable for the condition." You are combining the ageText truthiness check with JSX inline; introduce a separate boolean or value variable outside the JSX and then use it for conditional rendering.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still mandatory checklist violations, even though your functionality is correct. In src/components/Person/Person.jsx, the ternary expressions for partnerSex and ageText are written on a single line, which conflicts with [CHECKLIST ITEM #2]; you need to split the condition, ? branch, and : branch onto separate lines, for example:
const partnerSex = person.sex === 'm'
? 'wife'
: 'husband';Similarly, reformat ageText so its ternary follows the same multi-line style. Also in Person.jsx, the JSX conditional {shouldShowAge && <p className="Person__age">{ageText}</p>} still packs the condition and rendered element together, which goes against [CHECKLIST ITEM #3]; simplify this by keeping JSX driven by a straightforward boolean like shouldShowAge and avoid tying it directly to the ternary result. Once these checklist styling issues are fixed, your solution will fully meet both the functional and required code-style standards—overall you’re very close, and the changes are small but necessary for completion.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| @@ -1,5 +1,6 @@ | |||
| import React from 'react'; | |||
| import './App.scss'; | |||
There was a problem hiding this comment.
This line violates [CHECKLIST ITEM #2]: "Readabily is everything. Format ternary operator operands correctly - move each operand to the separate line". The condition, ? value, and : value should each be on their own line (compare with the GOOD EXAMPLE in the checklist).
| import { Person } from './components/Person/Person'; | ||
|
|
||
| export const misha = { | ||
| name: 'Misha', |
There was a problem hiding this comment.
This line also violates [CHECKLIST ITEM #2] because the ternary is kept on a single line instead of splitting condition, ? branch, and : branch onto separate lines as required by the GOOD EXAMPLE.
| @@ -1 +1,16 @@ | |||
| // export const Person = ({ person }) => (); | |||
| export const Person = ({ person }) => { | |||
| const partnerSex = person.sex === 'm' ? 'wife' : 'husband'; | |||
There was a problem hiding this comment.
This line violates checklist item #2: Format ternary operator operands correctly – move each operand to a separate line. partnerSex should be written so the condition, ? value, and : value are each on their own line.
| const partnerText = person.isMarried | ||
| ? `${person.partnerName} is my ${partnerSex}` | ||
| : 'I am not married'; | ||
| const ageText = person.age ? `I am ${person.age}` : ''; |
There was a problem hiding this comment.
This ternary also violates checklist item #2: Format ternary operator operands correctly – move each operand to a separate line. The condition, ? branch, and : branch for ageText need to be split across separate lines for readability.
| return ( | ||
| <div className="Person"> | ||
| <h2 className="Person__name">My name is {person.name}</h2> | ||
| {shouldShowAge && <p className="Person__age">{ageText}</p>} |
There was a problem hiding this comment.
This JSX conditional rendering still conflicts with checklist item #3: Avoid putting several cases to conditional rendering. Create a separate variable for the condition. You already have shouldShowAge; consider keeping JSX minimal by using a simple {shouldShowAge && ...} based on a straightforward boolean condition, rather than tying it to ageText and the ternary above in a single expression.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you learn, even though there is still a small style issue to improve. Functionally, your Person component and its usage in App.jsx are spot on: you pass the full person object, handle optional age correctly, and render partner information and marriage status exactly as required. You also did well applying destructured props and formatting all ternary operators across multiple lines as requested in the checklist.
The remaining improvement is around conditional rendering in src/components/Person/Person.jsx [CHECKLIST ITEM #3]: right now shouldShowAge is based on ageText, which itself comes from a ternary, so the JSX condition is indirectly tied to a formatted string instead of the raw data. On the line where you define shouldShowAge, try basing it directly on the source property, for example const shouldShowAge = Boolean(person.age);, so the condition is a simple, explicit boolean. Similarly, instead of {shouldShowAge && <p className="Person__age">{ageText}</p>}, you can precompute an ageElement variable (e.g., set it to the <p> when shouldShowAge is true, or null otherwise) and then render {ageElement} in JSX; this keeps the condition and the rendered element clearly separated.
Overall, you’re very close to a fully checklist-compliant solution, and these last tweaks will help you build even cleaner, more maintainable React components. Keep up the good work—your progress here shows solid understanding and careful attention to both functionality and style.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| const ageText = person.age | ||
| ? `I am ${person.age}` | ||
| : ''; | ||
| const shouldShowAge = Boolean(ageText) |
There was a problem hiding this comment.
This still violates checklist item #3: "[CODE STYLE] - Avoid putting several cases to conditional rendering. Create separate variable for the condition." Right now shouldShowAge is derived from ageText, which itself comes from a ternary; the condition in JSX is therefore indirectly tied to the ternary result. Consider making shouldShowAge a straightforward boolean based directly on the source property (e.g., Boolean(person.age)), so the JSX conditional is driven by a simple, explicit condition rather than the outcome of a formatted text value.
DEMO LINK