Skip to content

Fix Person component - #3865

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

Fix Person component#3865
loleksandra10-ops wants to merge 1 commit into
mate-academy:masterfrom
loleksandra10-ops:develop

Conversation

@loleksandra10-ops

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 required functionality works: props are correctly destructured, all three persons render as expected, age and marital status are handled correctly, and class names match the requirements. There are, however, two style issues to address for readability and checklist compliance.

  • [CHECKLIST ITEM #2] In src/components/Person/Person.jsx, the ternary operator is on a single line; each operand should be on its own line (e.g., split condition, truthy, and falsy branches across separate lines) for clarity.
  • [CHECKLIST ITEM #3] Also in Person.jsx, avoid nested ternaries for conditional rendering; extract the inner sex === 'm' ? 'wife' : 'husband' into a variable like const partnerType = ... and then use {partnerName} is my {partnerType}.
    These are non-blocking style fixes; the core behavior is solid. Nice work—just make those readability adjustments to align 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 on lines +11 to +13
{isMarried ? (
<p className="Person__partner">
{partnerName} is my {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.

Checklist item #2 violation: Ternary operator operands should each be on a separate line for readability. Currently all operands are on the same line as the condition.

Comment on lines +11 to +13
{isMarried ? (
<p className="Person__partner">
{partnerName} is my {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.

Checklist item #3 violation: Avoid putting several cases to conditional rendering. The inner ternary sex === 'm' ? 'wife' : 'husband' should be extracted to a separate variable (e.g., const partnerType = sex === 'm' ? 'wife' : 'husband';) and then used as {partnerName} is my {partnerType}.

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