Skip to content

disable prettier#3780

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

disable prettier#3780
sofiiamuzyka wants to merge 2 commits into
mate-academy:masterfrom
sofiiamuzyka:develop

Conversation

@sofiiamuzyka

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 it fully meets the functional and checklist requirements of the task. Your App.jsx correctly keeps the misha, olya, and alex objects intact and uses the Person component three times, passing the full person object via person={...} as required. In Person.jsx, you render the correct structure and class names, handle the optional age field properly by omitting it when absent, and implement the marital status and partner title logic correctly for both men and women, as well as the “I am not married” case. You also follow the prop destructuring requirement and format the ternary operator across multiple lines as requested.

The only remaining issue is a linter error (no-nested-ternary) caused by a nested ternary expression around line 10 in Person.jsx, but this is a style concern rather than a functional blocker and can be resolved by refactoring that logic into if statements or a helper variable. Overall, your implementation is correct, readable, and aligned with the task’s expectations—nice work.


✨ 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

@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 behavior is correct, it’s used three times in App, props are passed as full person objects, and the marital status and partner text render exactly as specified. The remaining issue is purely stylistic: the nested ternary inside the partner template literal in src/components/Person/Person.jsx (around the marital-status text) still violates the guideline about ternary formatting and previously triggered a no-nested-ternary linter warning. However, per the decision criteria, this is a code-style concern rather than a critical blocker, and it doesn’t break functionality or task requirements, so it doesn’t prevent approval. To improve the code, consider extracting the partner role ('wife' vs 'husband') into a separate variable or a small if block before the JSX, then interpolate that single value, which will both satisfy [CHECKLIST ITEM #2] and keep the markup more readable. Overall, you’ve done solid work implementing the logic correctly; cleaning up that conditional will be a good next step for style and maintainability.


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

Footnotes

  1. Rate AI review example

<p className="Person__partner">
{!person.isMarried
? `I am not married`
: `${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 violates checklist item #2: "Format ternary operator operands correctly - move each operand to the separate line" because you have a nested ternary person.sex === 'm' ? 'wife' : 'husband' inline inside a template literal. Consider extracting the partner role into a separate variable or using an if block so that any ternary you keep is formatted with each operand on its own line.

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