task solution#1 #3553
task solution#1 #3553
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on the implementation! Your components are functionally correct and meet almost all the requirements. However, one change is needed before this can be approved.
In src/components/Person/Person.jsx, the nested ternary operator used to determine the partner's title is written on a single line. This violates a specific project requirement: each operand of a ternary operator must be on a new line to improve readability [CHECKLIST ITEM #2].
Please format the ternary operator across multiple lines or extract the logic into a separate variable. Once you've made that adjustment, it will be ready for 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
src/components/Person/Person.jsx
Outdated
|
|
||
| <p className="Person__partner"> | ||
| {isMarried | ||
| ? `${partnerName} is my ${sex === 'm' ? 'wife' : 'husband'}` |
There was a problem hiding this comment.
This nested ternary operator should be formatted across multiple lines to improve readability, as required by checklist item #2. Consider extracting this logic into a separate variable before the return statement.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job addressing the previous feedback and getting so close to the solution! I am requesting one final adjustment.
In src/components/Person/Person.jsx, the ternary operator for the partnerTitle constant needs to be formatted across multiple lines, just like you did for partnerText. This is a required code style for the project [CHECKLIST ITEM #2].
Once you apply this formatting change, your work will be perfect. You're almost there!
✨ 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
src/components/Person/Person.jsx
Outdated
| export const Person = ({ person }) => { | ||
| const { name, age, sex, isMarried, partnerName } = person; | ||
|
|
||
| const partnerTitle = sex === 'm' ? 'wife' : 'husband'; |
There was a problem hiding this comment.
This is a great improvement in formatting the ternary operators! You've correctly formatted the one for partnerText. However, this one also needs to be split across multiple lines to meet the code style requirements.
This violates checklist item #2: 'Format ternary operator operands correctly - move each operand to the separate line'.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent job addressing the previous feedback! I am approving this solution to help you move forward, but there are a couple of small but important details to review for the future.
In src/components/Person/Person.jsx, the logic for partnerTitle is currently broken due to a small syntax mix-up. The ternary operator was wrapped in backticks (`), which turns the entire conditional into a single string. To fix this, you just need to remove the backticks and ensure the condition is a valid JavaScript comparison (e.g., sex === 'm').
Additionally, please remember to format all ternary operators across multiple lines for better readability, as required by the style guide [CHECKLIST ITEM #2]. You've done this correctly in other places, so just apply that same pattern to the partnerText variable.
You're doing great work. Keep up the momentum!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| sex === | ||
| `'m' | ||
| ? 'wife' | ||
| : 'husband'`; |
There was a problem hiding this comment.
It looks like there was a small mix-up while trying to format this ternary operator. The condition sex === is incomplete, and the rest of the logic ('m' ? 'wife' : 'husband') has been placed inside a single template literal string. This means sex is being compared to a long string, which isn't the goal.
A ternary operator needs a full condition (e.g., sex === 'm'), followed by the ? operator, the "if true" value, the : operator, and the "if false" value. You've already formatted the partnerText ternary perfectly—you can apply that same pattern here!
2 participants
No description provided.