Skip to content

sol#3906

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

sol#3906
Artur2912 wants to merge 2 commits into
mate-academy:masterfrom
Artur2912:develop

Conversation

@Artur2912
Copy link
Copy Markdown

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

Decision: Request Changes

Your implementation is close, but there are critical blockers that prevent the task from meeting the requirements and likely cause tests to fail.

Critical issues to fix:

  • src/App.jsx: Update the five rendered cases to match the exact requirements:
    1. Line X: Use <Sum a={2} b={3} /> (currently incorrect values).
    2. Line Y: Keep <Sum a={-5} b={5} /> (this one is correct).
    3. Line Z: Change <Sum a={10} b={0} /> to <Sum a={10} /> to test default b.
    4. Line A: Change <Sum a={0} b={5} /> to <Sum b={5} /> to test default a.
    5. Line B: Change <Sum a={0} b={0} /> to <Sum /> to test both defaults.
  • src/components/Sum/Sum.jsx: Implement defaulting of props (a and b should default to 0 when not provided). For example, destructure with defaults: const Sum = ({ a = 0, b = 0 }) => ....
  • src/components/Sum/Sum.jsx: Render a paragraph element (<p>) instead of a <div> as required.
  • src/components/Sum/Sum.jsx: Build the text using a single template literal for readability and to satisfy the style checklist, e.g. Sum of ${a} and ${b} is ${a + b} [CHECKLIST ITEM #2].

What’s good: the component structure is simple and the negative-number case is correctly included. Please apply the above fixes; they directly align with the task’s functional requirements and code-style checklist. Nice work so far—once updated, this should pass the 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 Outdated
<p>Sum of 0 and 0 is 0</p>
{/* Replace paragraphs with Sum componets */}
{/* And remove commented lines :) */}
<Sum a={3} b={5} />
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 renders the first Sum with incorrect values. The task requires a Sum with a = 2 and b = 3. Update this to:

<Sum a={2} b={3} />

Reference requirement: "a = 2 and b = 3;" from the description.

Comment thread src/App.jsx
{/* Replace paragraphs with Sum componets */}
{/* And remove commented lines :) */}
<Sum a={3} b={5} />
<Sum a={-5} b={5} />
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 is correct — it matches the required case a = -5 and b = 5. No change needed. Requirement: "a = -5 and b = 5;"

Comment thread src/App.jsx Outdated
{/* And remove commented lines :) */}
<Sum a={3} b={5} />
<Sum a={-5} b={5} />
<Sum a={10} b={0} />
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 currently passes b={0} in addition to a={10}. The task requires a Sum with just a = 10 (i.e., omit the b prop so Sum's defaulting behavior can apply). Change to:

<Sum a={10} />

Reference requirement: "just a = 10;" from the description.

Comment thread src/App.jsx Outdated
<Sum a={3} b={5} />
<Sum a={-5} b={5} />
<Sum a={10} b={0} />
<Sum a={0} b={5} />
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 currently passes a={0} together with b={5}. The task requires a Sum with just b = 5 (omit the a prop). Change to:

<Sum b={5} />

Reference requirement: "just b = 5;" from the description.

Comment thread src/App.jsx Outdated
<Sum a={-5} b={5} />
<Sum a={10} b={0} />
<Sum a={0} b={5} />
<Sum a={0} b={0} />
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 passes a={0} and b={0} but the task requires a Sum with no params at all. Change to:

<Sum />

Reference requirement: "no params at all." from the description.

Comment thread src/components/Sum/Sum.jsx Outdated
// export const Sum = () => ();
import React from 'react';

export const Sum = props => (
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 declares the component but it doesn't apply default values for a and b. The task requires: "If the a or b props are not passed, set them to 0." Consider destructuring with defaults (const Sum = ({ a = 0, b = 0 }) => ...) or using defaultProps / nullish coalescing before rendering. See description: "If the a or b props are not passed, set them to 0."

Comment thread src/components/Sum/Sum.jsx Outdated
import React from 'react';

export const Sum = props => (
<div>
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 is the opening of the rendered markup. The task requires rendering a paragraph element (<p>) with the result text (see description). Replace the <div> with a <p> so the output matches the requirement: "rendering a paragraph with a text like Sum of 2 and 1 is 3."

Comment thread src/components/Sum/Sum.jsx Outdated

export const Sum = props => (
<div>
Sum of {props.a} and {props.b} is {props.a + props.b}
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 builds the output text using multiple JSX fragments (Sum of {props.a} and {props.b} is {props.a + props.b}), which violates the code-style checklist. The checklist requires: "2. [CODE STYLE] - Use string interpolation inside tag content" (example: <p>{Sum of ${a} and ${b} is ${a + b}}</p>). Change this to a single template literal inside the tag content, e.g. {Sum of ${a} and ${b} is ${a + b}}.

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

Decision: APPROVE

Your implementation meets the task requirements. The Sum component is a named export, uses destructuring defaults for a and b (both default to 0), and renders the exact required <p> text via a single template literal expression. In App.jsx, all five required usages are present and the short JSX tags are kept on one line, aligning with the style checklist. No functional or syntax issues were found.

Minor suggestions (non-blocking):

  • Run the test suite (npm test) to confirm everything passes in your environment.
  • If required by the task, ensure your PR includes the correctly formatted demo link to GitHub Pages.

Great job keeping the code clean and concise!


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

Footnotes

  1. Rate AI review example

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