Skip to content

Add solution#3789

Open
apanyutin wants to merge 3 commits into
mate-academy:masterfrom
apanyutin:solution
Open

Add solution#3789
apanyutin wants to merge 3 commits into
mate-academy:masterfrom
apanyutin:solution

Conversation

@apanyutin

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 direct violations of the explicit checklist items, which are considered critical for this task. In src/components/Person/Person.jsx, the ternary that computes partnerString must be formatted with each operand on its own line, with the ? and : branches properly aligned and indented as required by [CHECKLIST ITEM #2] (e.g., break after the condition and put each result on a separate line). Also in Person.jsx, the JSX conditional person.age && <p className="Person__age">... violates [CHECKLIST ITEM #3], since the condition and JSX are inlined together; you should introduce a separate boolean like const hasAge = Boolean(person.age); and then use hasAge && ... in the JSX or similar, so the condition is clearly separated from the markup. Functionally, your Person component logic (destructured person prop, conditional age rendering, and marital status text based on sex and isMarried) looks correct and matches the task description.

To fix the critical issues:

  • Reformat the ternary assigning partnerString so that the condition, ? branch, and : branch each appear on their own lines with consistent indentation.
  • Extract the age condition into a separate variable before the return and use that variable inside JSX instead of putting person.age && ... directly.

Once you address these style checklist violations, your solution should fully meet both the functional and code-style requirements; the core idea and structure of your component are solid, so you are very close.


✨ 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 +7
partnerString = person.sex === 'f'
? `${person.partnerName} is my husband`
: `${person.partnerName} is my wife`;

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 ternary formatting violates checklist item #2: "Format ternary operator operands correctly - move each operand to the separate line". Align it like in the GOOD EXAMPLE, with ? and : branches on their own lines and properly indented.

Comment thread src/components/Person/Person.jsx Outdated
Comment on lines +13 to +14
{person.age &&
<p className="Person__age">I am {person.age}</p>}

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 JSX conditional rendering violates checklist item #3: "Avoid putting several cases to conditional rendering. Create separate variable for the condition.". Instead of person.age && ... inside JSX, create a separate boolean (e.g. const hasAge = Boolean(person.age);) and use that variable in the 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

I am requesting changes because there is still a direct violation of the explicit checklist requirement for conditional rendering, which counts as a critical blocking issue for this task. In src/components/Person/Person.jsx, the age is currently rendered with an inline condition like person.age && <p className="Person__age">..., which mixes the condition and JSX and goes against [CHECKLIST ITEM #3]. The checklist explicitly requires extracting such conditions into a separate variable before the return, for example:

const hasAge = Boolean(person.age);

return (
  <div className="Person">
    {hasAge && (
      <p className="Person__age">I am {person.age}</p>
    )}
    {/* ...other JSX... */}
  </div>
);

Functionally, your Person component looks solid: it uses the person prop correctly, formats the partner text as required, and uses the correct class names. The only blocker is the style checklist violation above; once you extract the age condition into a separate boolean and use that in JSX, your solution will align with both the functional requirements and the enforced code-style rules. You're very close—fixing this one item should be enough to get this task accepted.


✨ 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
return (
<section className="Person">
<h2 className="Person__name">My name is {person.name}</h2>
{person.age && <p className="Person__age">I am {person.age}</p>}

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 #3: Avoid putting several cases to conditional rendering. Create separate variable for the condition. You currently inline person.age && <p ...> with the markup; instead, introduce a separate boolean (e.g., const hasAge = Boolean(person.age);) before the return and then use hasAge && ... in JSX so the condition is clearly separated from the element.

@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 approving your solution as an educational override so you can keep progressing while you learn, even though there is still one style issue to address. Functionally, your Person component looks solid: it correctly uses the person prop object, handles married/unmarried states, uses the right class names, and makes age optional.

The remaining issue is about conditional rendering style for the age: in src/components/Person/Person.jsx, you currently use hasAge && <p className="Person__age">I am {person.age}</p> directly in JSX, which conflicts with [CHECKLIST ITEM #3]. Instead, before the return, create a separate variable like const ageElement = hasAge ? (<p className="Person__age">I am {person.age}</p>) : null; and then just render {ageElement} inside your JSX. This keeps logic and markup a bit cleaner and matches the required style pattern for conditional rendering. Overall, you’ve done a good job meeting the functional requirements; adjusting this small style detail will bring your code fully in line with the checklist and help you build better habits going forward.


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

Footnotes

  1. Rate AI review example

Comment thread src/App.scss
@@ -14,7 +14,7 @@ iframe {
}

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 #3: "[CODE STYLE] - Avoid putting several cases to conditional rendering. Create separate variable for the condition." Even though you created hasAge, you still mix the condition and JSX inline with {hasAge && <p className="Person__age">I am {person.age}</p>}. Extract the conditional JSX into a separate variable before the return and then render that variable here.

return (
<section className="Person">
<h2 className="Person__name">My name is {person.name}</h2>
{hasAge && <p className="Person__age">I am {person.age}</p>}

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 #3: "[CODE STYLE] - Avoid putting several cases to conditional rendering. Create separate variable for the condition." You are still using hasAge && <p ...> directly in JSX. Follow the previous review example by creating a separate variable (e.g. const ageElement = hasAge ? (...) : null;) before the return and then render that variable inside JSX.

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