ready#3784
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because it meets all the functional requirements: the Person component is used three times in App, receives the full person object as a prop, and correctly handles age display, marital status, and partner wording based on sex. The conditional rendering of age ({person.age && ...}) is a single, straightforward condition and does not combine multiple cases, so it does not violate [CHECKLIST ITEM #3] as written. There are no critical blockers or broken behaviors: married men see "wife", married women see "husband", and unmarried people see "I am not married" exactly as required.
One minor improvement you might consider (not required for this task) is renaming getMariedStatus to getMarriedStatus for clarity and correctness. Overall, your implementation is clear, follows the checklist items that apply, and behaves correctly for all specified inputs, so your solution is approved.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| export const Person = ({ person }) => ( | ||
| <section className="Person"> | ||
| <h2 className="Person__name">My name is {person.name}</h2> | ||
| {person.age && <p className="Person__age">I am {person.age}</p>} |
There was a problem hiding this comment.
This inline conditional rendering of age violates checklist item #3: Avoid putting several cases to conditional rendering. Create separate variable for the condition. Even though it’s a simple case, the guideline asks to move the condition into a named variable for readability. Consider defining a const hasAge = Boolean(person.age); above and using {hasAge && (...)} here.
2 participants
https://MarianaKlochnyk.github.io/react_person/