Skip to content

react-person#3802

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

react-person#3802
Tendeeer wants to merge 1 commit into
mate-academy:masterfrom
Tendeeer:develop

Conversation

@Tendeeer

Copy link
Copy Markdown

@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 renders the right markup, is used three times in App, receives the whole person object as a prop, handles optional age, and correctly shows marriage status and partner wording for both sexes. The only issue identified is a style / checklist point around ternary formatting for partnerSex in both src/App.jsx and src/components/Person/Person.jsx [CHECKLIST ITEM #2], where the condition, ? branch, and : branch should each be on their own line. This is important for readability, but it does not break functionality or the task’s core requirements, so it is not a blocker. When you revisit the code, you can fix it by rewriting it in the form:

const partnerSex = person.sex === 'f'
  ? 'husband'
  : 'wife';

Overall, your implementation is solid and aligned with the task; just keep an eye on these formatting details in future tasks to stay consistent with the checklist.


✨ 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';
import './App.scss';

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: Readabily is everything. Format ternary operator operands correctly - move each operand to the separate line. Here the ternary for partnerSex is on one line; update it so the condition, ? branch, and : branch are each on their own lines.

@@ -1 +1,14 @@
// export const Person = ({ person }) => ();
export const Person = ({ person }) => {
const partnerSex = person.sex === 'f' ? 'husband' : '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 violates checklist item #2: "Readabily is everything. Format ternary operator operands correctly – move each operand to the separate line". Your partnerSex assignment uses the bad example pattern (condition ? firstValue : secondValue) instead of:

const partnerSex = person.sex === 'f'
  ? 'husband'
  : 'wife';

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