Skip to content

add task solution#3864

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

add task solution#3864
Augustwise wants to merge 3 commits into
mate-academy:masterfrom
Augustwise:develop

Conversation

@Augustwise
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 — critical blockers present.

Why: The implementation doesn’t follow the required prop names or formatting, which will fail automated checks and break the contract between components.

Issues to fix (critical blockers):

  1. src/components/Sum/Sum.jsx — Wrong prop names and formatting

    • Problem (line numbers approximate):
      • Line 1-3: Component signature uses number1/number2 instead of a/b (requirement: Sum must accept a and b, default to 0 if missing).
      • Paragraph content splits the sentence into multiple JSX expressions, violating the checklist item requiring a single template-string interpolation.
    • Why: Tests expect a/b and a single template string like the provided example.
    • How to fix: Change signature to export const Sum = ({ a = 0, b = 0 }) => ... and the content to <p>{Sum of ${a} and ${b} is ${a + b}}</p>.
  2. src/App.jsx — Incorrect prop names in all Sum usages

    • Problem (multiple lines): Passing number1/number2 props.
    • Why: App must render five instances with exact props using a/b as specified.
    • How to fix: Update all five usages to:
      • <Sum a={2} b={3} />
      • <Sum a={-5} b={5} />
      • <Sum a={10} />
      • <Sum b={5} />
      • <Sum />

Minor notes: Your component export style and providing defaults are good starts. After making the above changes, run tests to confirm compliance. Nice work so far—these are quick, targeted fixes that will bring everything in line with the requirements.


✨ 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 number1={2} number2={3} />
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 usage violates the task description. The description requires App to render a Sum instance with a = 2 and b = 3. Replace these props with the required names so the component receives the expected values. Expected: <Sum a={2} b={3} />. See the description: "The App should contain the next Sum components: - a = 2 and b = 3;"

Comment thread src/App.jsx Outdated
{/* Replace paragraphs with Sum componets */}
{/* And remove commented lines :) */}
<Sum number1={2} number2={3} />
<Sum number1={-5} number2={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 usage violates the task description. The description requires App to render a Sum instance with a = -5 and b = 5. Use the prop names a and b: <Sum a={-5} b={5} />. See the description bullet list.

Comment thread src/App.jsx Outdated
{/* And remove commented lines :) */}
<Sum number1={2} number2={3} />
<Sum number1={-5} number2={5} />
<Sum number1={10}/>
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 usage violates the task description. The description requires an instance with just a = 10. Change the prop name to a: <Sum a={10} />. See the required App usages in the description.

Comment thread src/App.jsx Outdated
<Sum number1={2} number2={3} />
<Sum number1={-5} number2={5} />
<Sum number1={10}/>
<Sum number2={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 usage violates the task description. The description requires an instance with just b = 5. Change the prop name to b: <Sum b={5} />. See the required App usages in the description.

Comment thread src/components/Sum/Sum.jsx Outdated
@@ -1 +1,3 @@
// export const Sum = () => ();
export const Sum = ({number1 = 0, number2 = 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 uses different prop names than required. The task description explicitly says: "Create a Sum component accepting a and b props and rendering a paragraph... If the a or b props are not passed, set them to 0." Please change the signature to use a and b with defaults, e.g. export const Sum = ({ a = 0, b = 0 }) => ... so it matches the specification and will receive values correctly from App.jsx.

Comment thread src/components/Sum/Sum.jsx Outdated
// export const Sum = () => ();
export const Sum = ({number1 = 0, number2 = 0}) => (
<p>Sum of {number1} and {number2} is {number1 + number2}</p>
); No newline at end of file
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 violates checklist item 2 which requires using a single template-string expression inside the tag content. Exact checklist text: "2. [CODE STYLE] - Use string interpolation inside tag content". The GOOD EXAMPLE shows: <p>{Sum of ${a} and ${b} is ${a + b}}</p>. Please replace the current fragments with that single template expression (and use a/b as prop names).

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: Requesting changes due to a checklist violation that is explicitly required.

  • The component is functionally correct (defaults, named export, correct sum, correct App usage), but there is a CRITICAL BLOCKER against the explicit checklist requirement: “Use string interpolation inside tag content.”
  • Location: src/components/Sum/Sum.jsx, line with the <p> element. Currently it’s written as: Sum of {a} and {b} is {a + b} which mixes literal text with multiple JSX expressions.
  • Why this is wrong: The checklist mandates a single template-string expression inside the tag content, not multiple interpolations.
  • How to fix: Replace the paragraph content with a single template literal, e.g. <p>{Sum of ${a} and ${b} is ${a + b}}</p>.
  • Minor note: Everything else (props defaults, named export, App rendering) looks good—nice work on those. Make this one-line change, then run tests/formatting to finalize.

✨ 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/components/Sum/Sum.jsx Outdated
// export const Sum = () => ();
export const Sum = ({ a = 0, b = 0 }) => (
<p>
Sum of {a} and {b} is {a + 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 violates the checklist item #2 exactly: "2. [CODE STYLE] - Use string interpolation inside tag content"

Currently the paragraph is written as:

Sum of {a} and {b} is {a + b}

Change it to use a single template-string expression inside the tag, for example:

{`Sum of ${a} and ${b} is ${a + b}`}

This matches the GOOD EXAMPLE in the checklist and satisfies the implementation detail requiring string interpolation (see checklist) .

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

Educational approval: Proceeding to approve so you can keep moving forward while learning.

What you did well:

  • Sum is a named export, implemented as a concise functional component with sensible defaults: ({ a = 0, b = 0 }).
  • App.jsx correctly imports Sum by name and renders all five required instances on single lines.

What needs improvement (critical):

  • src/components/Sum/Sum.jsx line with the <p> currently renders literal ${a}, ${b}, and ${a + b}. This violates checklist item #2: [CODE STYLE] - Use string interpolation inside tag content and causes incorrect output. The issue occurs because the template placeholders are not inside a single JavaScript expression wrapped in { }.
  • How to fix: Replace the paragraph content with a single template-literal expression inside JSX braces, e.g., <p>{Sum of ${a} and ${b} is ${a + b}}</p>. This ensures proper interpolation and matches the checklist requirement.

Minor tip:

  • After updating, run npm test to verify the rendered text matches expectations. Keep going—you’re very close to perfect here!

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

Footnotes

  1. Rate AI review example

// export const Sum = () => ();
export const Sum = ({ a = 0, b = 0 }) => (
<p>
Sum of ${a} and ${b} is ${a + 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 violates checklist item #2: "[CODE STYLE] - Use string interpolation inside tag content"

The current line renders the literal characters ${a} and ${b} instead of interpolating values. In src/components/Sum/Sum.jsx change the paragraph to a single template-literal expression inside JSX braces. For example:

<p>{`Sum of ${a} and ${b} is ${a + b}`}</p>

This will interpolate the numbers and satisfy the checklist requirement.

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