Skip to content

add solution#3786

Open
m-berezovskyy wants to merge 2 commits into
mate-academy:masterfrom
m-berezovskyy:develop
Open

add solution#3786
m-berezovskyy wants to merge 2 commits into
mate-academy:masterfrom
m-berezovskyy:develop

Conversation

@m-berezovskyy

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because there are still explicit checklist violations in src/components/Person/Person.jsx, even though the functionality is correct. First, per [CHECKLIST ITEM #1], you only destructure { person } from props and then repeatedly use person.name, person.age, etc.; instead, destructure these fields at the top of the component, for example const { name, age, sex, isMarried, partnerName } = person;, and then use name, age, etc. directly in JSX. Second, per [CHECKLIST ITEM #2], the nested ternary that decides between wife and husband, and the outer ternary around it, currently have operands on the same line; reformat them so the condition, ? branch, and : branch are each on their own line for better readability and checklist compliance. Third, per [CHECKLIST ITEM #3], you have a complex conditional expression for the partner text directly in JSX; extract this into a separate variable like const partnerText = ... above the return, using the properly formatted ternaries and isMarried/sex logic, and then render {partnerText} inside the Person__partner element. Functionally your solution meets the task requirements (three Person usages in App, correct age omission, spouse naming, and "I am not married" for the unmarried case), so once these style and checklist issues are fixed, your solution will be fully acceptable. Overall, you’re very close; cleaning up the destructuring and conditional rendering will make the code clearer and fully aligned with the required checklist.


✨ 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. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/components/Person/Person.jsx Outdated
Comment on lines +5 to +8
export const Person = ({ person }) => {
return (
<section className="Person">
<h2 className="Person__name">{`My name is ${person.name}`}</h2>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #1: Use destructuring for getting access to values of props object. You destructure person from props, but then repeatedly use person.name, person.age, etc. inside JSX instead of destructuring those fields from person.

Comment thread src/components/Person/Person.jsx Outdated
Comment on lines +13 to +15
{person.isMarried
? `${person.partnerName} is my ${person.sex === 'm' ? 'wife' : 'husband'}`
: 'I am not married'}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #2: Format ternary operator operands correctly - move each operand to the separate line. The nested ternary person.sex === 'm' ? 'wife' : 'husband' and the surrounding ternary are written with operands on the same line instead of each operand on its own line.

Comment thread src/components/Person/Person.jsx Outdated
Comment on lines +12 to +15
<p className="Person__partner">
{person.isMarried
? `${person.partnerName} is my ${person.sex === 'm' ? 'wife' : 'husband'}`
: 'I am not married'}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a complex conditional rendering expression with multiple cases in JSX, which matches the kind of pattern forbidden by checklist item #3: Avoid putting several cases to conditional rendering. Create separate variable for the condition. Consider computing the partner description or message in a variable before returning JSX.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Your solution is approved because it correctly implements all the required functionality for the Person component and its usage in App. The component handles optional age, marital status, and spouse wording (wife/husband based on sex) correctly, and the unmarried case shows the expected “I am not married” message. You consistently pass the full person object via the person prop, destructure it in the component, and keep the misha, olya, and alex constants unchanged as required. Class names in JSX match the specified styles (Person, Person__name, Person__age, Person__partner), so the styling contract is preserved. Previous checklist concerns (prop destructuring, simplifying complex conditional rendering, and moving partner logic into a separate variable) have been addressed and no remaining issues are blocking. As a minor style improvement for the future, keep an eye on ternary formatting for readability, following the multi-line guideline when conditionals get more complex [CHECKLIST ITEM #2]. Overall, this is a clean, readable implementation that meets the task’s requirements well.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants