Skip to content

result#3745

Open
Zakys1d wants to merge 3 commits into
mate-academy:masterfrom
Zakys1d:develop
Open

result#3745
Zakys1d wants to merge 3 commits into
mate-academy:masterfrom
Zakys1d:develop

Conversation

@Zakys1d
Copy link
Copy Markdown

@Zakys1d Zakys1d commented May 20, 2026

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

The implementation has critical issues that will cause test failures. First, there's a module export mismatch - the test imports { Person } as a named export, but Person.jsx likely uses a default export instead. Second, the text formats don't match test expectations: the component renders "Misha", "Age: 37", and "Wife: Natasha", but tests expect "My name is Misha", "I am 37", and "Natasha is my wife". Additionally, there are two checklist violations: the ternary operator operands at line 10 need to be on separate lines [CHECKLIST ITEM #2], and the nested ternary should be extracted to a separate variable before the JSX [CHECKLIST ITEM #3].


✨ 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/App.jsx
age: 37,
sex: 'm',
isMarried: true,
partnerName: 'Natasha',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test expects text 'Natasha is my wife' but the component outputs 'Wife: Natasha'. Change format to '{partnerName} is my {wife/husband}'.

Comment thread src/App.jsx
age: 37,
sex: 'm',
isMarried: true,
partnerName: 'Natasha',
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. Current format: 'condition ? firstValue : secondValue' should be: 'condition ? firstValue : secondValue' with line breaks.

Comment thread src/components/Person/Person.jsx Outdated
const Person = ({ person }) => {
return (
<div className="Person">
<h2 className="Person__name">{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.

The name text should be 'My name is {name}' but currently renders just '{name}'. This will fail the test on line 20 of Person.spec.jsx.

Comment thread src/components/Person/Person.jsx Outdated
return (
<div className="Person">
<h2 className="Person__name">{person.name}</h2>
{person.age && <p className="Person__age">Age: {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.

The age text should be 'I am {age}' but currently renders 'Age: {age}'. This will fail the test on line 24 of Person.spec.jsx.

Comment thread src/components/Person/Person.jsx Outdated
{person.age && <p className="Person__age">Age: {person.age}</p>}
{person.isMarried ? (
<p className="Person__partner">
{person.sex === 'm' ? 'Wife' : 'Husband'}: {person.partnerName}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The partner text should be '{partnerName} is my {role}' (e.g., 'Natasha is my wife') but currently renders '{role}: {partnerName}' (e.g., 'Wife: Natasha'). This will fail tests on lines 28 and 44 of Person.spec.jsx.

Comment thread src/components/Person/Person.jsx Outdated
{person.age && <p className="Person__age">Age: {person.age}</p>}
{person.isMarried ? (
<p className="Person__partner">
{person.sex === 'm' ? 'Wife' : 'Husband'}: {person.partnerName}
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. The nested ternary should be formatted like: const partnerRole = person.sex === 'm' ? 'Wife' : 'Husband';

Comment thread src/components/Person/Person.jsx Outdated
{person.age && <p className="Person__age">Age: {person.age}</p>}
{person.isMarried ? (
<p className="Person__partner">
{person.sex === 'm' ? 'Wife' : 'Husband'}: {person.partnerName}
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. The nested ternary (person.sex === 'm' ? 'Wife' : 'Husband') should be extracted to a variable before the JSX.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

The implementation has two critical issues that must be fixed before approval:

  1. Import/export mismatch (src/App.jsx line 2): The Person component is exported as a named export (export const Person) but App.jsx imports it as a default import (import Person from). This causes an ESLint error (import/no-named-as-default) and will result in Person being undefined at runtime. Change line 2 to use a named import:

    import { Person } from './components/Person/Person';
  2. Ternary operator formatting (src/App.jsx line 4): Violates [CHECKLIST ITEM #2] - ternary operands should be on separate lines. Format as:

    const partnerRole = person.sex === 'm'
      ? 'wife'
      : 'husband';

Once these two issues are resolved, the code should pass all checks and tests.


✨ 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/App.jsx
import React from 'react';
import Person from './components/Person/Person';
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: ternary operator operands should be on separate lines. Format it as:

const partnerRole = person.sex === 'm'
  ? 'wife'
  : 'husband';

@@ -1,3 +1,4 @@
/* eslint-disable import/named */
import { mount } from '@cypress/react18';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import mismatch: App.jsx uses default import import Person from, but Person.jsx exports a named export export const Person. Change App.jsx line 2 to use named import: import { Person } from './components/Person/Person';

/* eslint-disable import/named */
import { mount } from '@cypress/react18';
import { Person } from './Person';

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: The ternary operator operands should be on separate lines. Format as:

const partnerRole = person.sex === 'm'
  ? 'wife'
  : 'husband';

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

This is an educational approval to help you progress while learning. Great job fixing the import/export mismatch from the previous review — your named imports are now correct! All functional requirements pass: the component renders properly, age is conditionally hidden, and the husband/wife logic works correctly.

Minor learning point: There's one formatting issue remaining — in Person.jsx line 4, the ternary operator should have each operand on its own line for readability [CHECKLIST ITEM #2]:


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

Footnotes

  1. Rate AI review example

import React from 'react';

export const Person = ({ person }) => {
const partnerRole = 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.

Violates CHECKLIST ITEM #2 - ternary operands should be on separate lines for readability. Format as:

const partnerRole = person.sex === 'm'
  ? 'wife'
  : 'husband';

Comment thread src/App.jsx
import React from 'react';
import { Person } from './components/Person/Person';
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.

Violates checklist item #2: ternary operands should be on separate lines. Format as:

/* eslint-disable import/named */
import { mount } from '@cypress/react18';
import { Person } from './Person';

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 operator matches the BAD EXAMPLE pattern from [CHECKLIST ITEM #2]. Format operands on separate lines:

const partnerRole = person.sex === 'm'
  ? 'wife'
  : 'husband';

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