Skip to content

add task solution#3787

Open
illyaSmyk wants to merge 1 commit into
mate-academy:masterfrom
illyaSmyk:develop
Open

add task solution#3787
illyaSmyk wants to merge 1 commit into
mate-academy:masterfrom
illyaSmyk:develop

Conversation

@illyaSmyk

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 approving your solution because all functional requirements are correctly implemented: the Person component behaves as expected for all three people, age is treated as optional, and marriage/partner wording and class names are correct. The remaining issues are checklist‑style concerns around destructuring and ternary formatting, but they do not break functionality or block task completion. To align better with [CHECKLIST ITEM #1], consider fully destructuring the person prop inside Person.jsx, e.g. const { name, age, sex, isMarried, partnerName } = person;, and then use name, age, etc. directly instead of person.name, person.age, and so on. For readability and [CHECKLIST ITEM #2] and [CHECKLIST ITEM #3], you can compute the partner role in a separate variable before JSX, for example const partnerRole = sex === 'm' ? 'wife' : 'husband';, and then use that variable in the template literal instead of an inline ternary. Overall, your logic is solid and matches the task requirements; the suggested changes are style improvements that will help you better follow the provided checklist and keep your React components cleaner 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.jsx
@@ -1,5 +1,6 @@
import React from 'react';

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 } but then repeatedly access person.name, person.age, etc. Instead, destructure the needed fields from person (e.g. const { name, age, sex, isMarried, partnerName } = person;) and use them directly.

Comment on lines +1 to +3
export const Person = ({ person }) => (
<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". Instead of accessing person.name, person.age, etc. directly from person, destructure the needed fields (name, age, sex, isMarried, partnerName) from the person prop.

<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 line uses person.age inline and relies on truthiness for conditional rendering. Combined with other inline person usages, it conflicts with checklist item #1, which requires destructuring props into separate variables.

Comment on lines +9 to +11
{person.isMarried && (
<p className="Person__partner">
{`${person.partnerName} is my ${person.sex === 'm' ? 'wife' : 'husband'}`}

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 line combines person.isMarried and a ternary inside a template literal: ${person.partnerName} is my ${person.sex === 'm' ? 'wife' : 'husband'}. It violates checklist item #2 (ternary operands should be on separate lines) and checklist item #3 (avoid several cases in conditional rendering; move the condition into a separate variable). Consider computing the role (wife/husband) in a variable before JSX and formatting the ternary across multiple lines.

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