react_person#3734
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your code structure is well-implemented with proper destructuring, correct ternary formatting, and appropriate class names. However, there's a critical bug causing test failure: the age display format is incorrect. The test expects the output to be "I am 37" but your component is rendering "age: 37". In your Person component, change the age paragraph from <p>age: {age}</p> to <p>I am {age}</p>. This is a functional issue that breaks the expected output format specified in the tests, so it must be fixed before approval.
✨ 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
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation correctly handles all functional requirements—conditional age display, proper partner terminology based on gender, "I am not married" messaging, correct class names, destructuring for props, and passing the entire person object as a prop. However, there's a formatting issue in src/components/Person/Person.jsx with the ternary operator used for determining partner terminology. The nested ternary inside the template literal ${sex === 'f' ? 'husband' : 'wife'} is written inline on a single line. Per checklist item #2, each ternary operand (condition, true branch, false branch) should be on its own line. To fix this, you can extract the partner logic into a separate constant before the JSX return: javascript const partnerText = person.sex === 'f' ? 'husband' : 'wife'; This way the nested ternary is no longer inline, and your component will fully comply with the formatting requirements while maintaining all functional behavior.
✨ 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
| {/* Тернарний оператор: якщо одружений — показуємо партнера, інакше — статус */} | ||
| {isMarried | ||
| ? `${partnerName} is my ${sex === 'f' ? 'husband' : 'wife'}` | ||
| : 'I am not married'} |
There was a problem hiding this comment.
Format ternary operator operands correctly - move each operand to the separate line. This violates checklist item #2. The condition, true branch, and false branch should each be on their own line.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution with an educational override, as all functional requirements are working correctly.
What you did well:
- All three Person components render correctly with proper destructuring
- Partner terminology logic works properly (husband/wife based on gender)
- Age display handles missing data correctly
- "I am not married" messaging displays when appropriate
- Class names match the requirements
Learning opportunity:
The nested ternary on line 19 (${sex === 'f' ? 'husband' : 'wife'}) is written inline. For better readability, consider extracting it to a constant before the return statement, like:
const partnerText = sex === 'f' ? 'husband' : 'wife';This follows the principle that complex expressions in JSX templates are often easier to understand when broken into named variables first.
Keep up the solid work on the core functionality—you've got the logic right!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| @@ -18,6 +18,7 @@ module.exports = defineConfig({ | |||
| }, | |||
| component: { | |||
There was a problem hiding this comment.
The nested ternary ${sex === 'f' ? 'husband' : 'wife'} is written inline within the template literal. Per checklist item #2, each ternary operand should be on its own line. Extract the partner terminology into a separate constant before the JSX return, e.g.: const partnerText = sex === 'f' ? 'husband' : 'wife';
| {isMarried | ||
| ? `${partnerName} is my ${sex === 'f' ? 'husband' : 'wife'}` |
There was a problem hiding this comment.
This nested ternary ${sex === 'f' ? 'husband' : 'wife'} violates checklist item #2 - all ternary operands should be on separate lines. Extract the partner terminology into a separate constant before JSX: const partnerText = sex === 'f' ? 'husband' : 'wife'; then use it in the template: ${partnerName} is my ${partnerText}
https://gadyka08-lab.github.io/react_person/