Skip to content

Develop#4229

Open
Tysiachnyk wants to merge 4 commits into
mate-academy:masterfrom
Tysiachnyk:develop
Open

Develop#4229
Tysiachnyk wants to merge 4 commits into
mate-academy:masterfrom
Tysiachnyk:develop

Conversation

@Tysiachnyk
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

I am requesting changes because a couple of core task requirements and explicit checklist items are still not met, which would cause tests/checks to fail.

Critical issues that must be fixed:

  1. In src/App.jsx, the Sum usages must match the exact required cases with omitted props: you should render Sum as:
    • <Sum a={2} b={3} />
    • <Sum a={-5} b={5} />
    • <Sum a={10} /> (no b prop)
    • <Sum b={5} /> (no a prop)
    • <Sum /> (no a or b)
      Currently, you pass explicit 0 values in some of these spots, which violates the requirement to omit props and will break the tests for those scenarios.
  2. In src/components/Sum/Sum.jsx, the paragraph content must use a single template string inside JSX as required by [CHECKLIST ITEM #2]. Replace the mixed JSX/text with something like:
    • <p>{Sum of ${a} and ${b} is ${a + b}}</p>
  3. The same string interpolation style is required wherever you show that text inside App.jsx (if you render such a paragraph there), so make sure any similar paragraphs use a single template literal rather than multiple {} expressions.

On the positive side, you correctly implemented Sum as a named export and set default values for a and b via destructuring, and your single-line JSX tags for short props already follow [CHECKLIST ITEM #1]. After you adjust the Sum usages to omit props where required and switch the paragraph(s) to use template string interpolation, your solution should fully meet the task 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
{/* And remove commented lines :) */}
<Sum a={2} b={3} />
<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 violates the task requirements. The App must include the following Sum components:

  • Sum with just a = 10 (no b prop)
  • Sum with just b = 5 (no a prop)
  • Sum with no params at all (neither a nor b)

In the current file you passed explicit zeros instead of omitting the props. Remove the unnecessary props so the components match the required cases. For example: <Sum a={10} />, <Sum b={5} />, and <Sum />.

Comment thread src/App.jsx
@@ -1,14 +1,13 @@
import React from 'react';
import './App.scss';
import { Sum } from './components/Sum/Sum';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Because you're required to use string interpolation inside tag content (checklist item #2), the paragraph should use a template string like {Sum of ${a} and ${b} is ${a + b}} instead of separate JSX expressions. Update the <p> content to use a single template interpolation.

Comment thread src/App.jsx
<p>Sum of 0 and 0 is 0</p>
{/* Replace paragraphs with Sum componets */}
{/* And remove commented lines :) */}
<Sum a={2} b={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.

Checklist item #1 asks that when a tag has fewer than 3 attributes and the values are short, the tag be written in one line. The existing <Sum a={2} b={3} /> and similar are already one-line — keep that style when you change the other Sum usages to omit props.

Comment thread src/components/Sum/Sum.jsx Outdated
Comment on lines +2 to +3
<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: Use string interpolation inside tag content. The paragraph currently uses plain JSX expressions mixed with text. Use a template string inside braces, for example: {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

The PR has a critical violation of CHECKLIST ITEM #2 in src/components/Sum/Sum.jsx. The paragraph currently uses separate JSX expressions ({a}, {b}, {a + b}) instead of the required single template literal interpolation. This is an explicit checklist requirement, not a style preference. The fix is straightforward: replace the paragraph content with <p>{Sum of ${a} and ${b} is ${a + b}}</p>. Everything else in your implementation is correct—the component accepts the correct props with default values, named export is proper, and App.jsx contains all 5 required Sum usages with properly omitted props. Once you update the template literal syntax in Sum.jsx, this will be ready for approval.


✨ 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
@@ -1,14 +1,13 @@
import React from 'react';
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 which requires using string interpolation (template literals) inside tag content. Replace with <p>{Sum of ${a} and ${b} is ${a + b}}</p> instead of multiple {} expressions.

Comment thread src/components/Sum/Sum.jsx Outdated
@@ -1 +1,3 @@
// export const Sum = () => ();
export const Sum = ({ a = 0, b = 0 }) => (
<p>Sum of {a} and {b} is {a + b}</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.

This violates [CHECKLIST ITEM #2] - use template literal interpolation inside the paragraph instead of separate JSX expressions. Replace with: <p>{Sum of ${a} and ${b} is ${a + b}}</p>

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

Your solution is approved! Great job fixing the template literal interpolation in Sum.jsx — using ${a}, ${b}, and ${a + b} inside the paragraph tag is exactly what was needed. You successfully implemented all requirements: the Sum component with default values, named export, five different prop combinations in App.jsx, and proper component styling. All checklist items are now passing. Keep up the excellent work!


✨ 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