first update#4340
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still a couple of critical requirement mismatches that affect both functionality and checklist compliance. In src/App.jsx, the last three Sum usages must rely on missing props defaulting to 0, but you currently pass explicit zeros: for the “just a = 10” case you should remove b={0} and render <Sum a={10} />, for the “just b = 5” case remove a={0} and render <Sum b={5} />, and for the “no params at all” case render <Sum /> instead of <Sum a={0} b={0} /> (these are required to demonstrate the default-prop behavior). In src/components/Sum/Sum.jsx, your <p> content still violates [CHECKLIST ITEM #2] – Use string interpolation inside tag content: instead of Sum of {a} and {b} is {a + b}, the requirement is to use a single template literal like:
<p>{`Sum of ${a} and ${b} is ${a + b}`}</p>These two changes are necessary for the solution to match the task description and the explicit checklist rules; once updated, the overall structure and logic you already have will fit the requirements well. Keep the rest of your implementation (named export, general structure) as is, since those parts look good.
✨ 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,14 +1,17 @@ | |||
| import React from 'react'; | |||
| import './App.scss'; | |||
| import { Sum } from './components/Sum/Sum'; | |||
There was a problem hiding this comment.
This implementation violates checklist item #2: "Use string interpolation inside tag content". Instead of mixing raw text and {a}/{b}/{c}, wrap the whole sentence in a single template string like the "GOOD EXAMPLE" in the checklist.
|
|
||
| <Sum a={-5} b={5} /> | ||
|
|
||
| <Sum a={10} b={0} /> |
There was a problem hiding this comment.
This usage does not match the description requirement for "just a = 10". You should omit b entirely and rely on the default prop value (0) instead of explicitly passing b={0}.
|
|
||
| <Sum a={10} b={0} /> | ||
|
|
||
| <Sum a={0} b={5} /> |
There was a problem hiding this comment.
This usage does not match the description requirement for "just b = 5". You should omit a entirely and rely on the default prop value (0) instead of explicitly passing a={0}.
|
|
||
| <Sum a={0} b={5} /> | ||
|
|
||
| <Sum a={0} b={0} /> |
There was a problem hiding this comment.
This usage does not match the description requirement for "no params at all". You should render <Sum /> without any props instead of explicitly passing a={0} and b={0}.
| // export const Sum = () => (); | ||
| export const Sum = ({ a = 0, b = 0, c = a + b }) => ( | ||
| <p> | ||
| Sum of {a} and {b} is {c} |
There was a problem hiding this comment.
This violates checklist item #2: "[CODE STYLE] - Use string interpolation inside tag content". Instead of splitting the sentence into multiple expressions (Sum of {a} and {b} is {c}), wrap the whole text in a template literal like {Sum of ${a} and ${b} is ${a + b}} inside the <p> tag.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is still a critical checklist violation in the Sum component’s paragraph content. The task explicitly requires [CHECKLIST ITEM #2] – Use string interpolation inside tag content, and the instructions call for a single template literal inside the <p> tag rather than multiple {} interpolations. In src/App.jsx and src/components/Sum/Sum.jsx, your paragraph currently looks like Sum of {a} and {b} is {a + b}, which breaks this requirement.
To fix this, update the JSX in src/components/Sum/Sum.jsx (and any duplicate in App.jsx if present) to:
<p>{`Sum of ${a} and ${b} is ${a + b}`}</p>This keeps the text as one continuous template literal, satisfying the checklist and the previous review comment. Everything else about your solution (named export, defaulting a and b to 0, and all required usages in App) looks correct, so once this single change is made your implementation should fully meet the task requirements. Nice work overall getting the functional behavior right; this last change is purely about matching the specified string interpolation style.
✨ 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,14 +1,17 @@ | |||
| import React from 'react'; | |||
| import './App.scss'; | |||
| import { Sum } from './components/Sum/Sum'; | |||
There was a problem hiding this comment.
This violates [CHECKLIST ITEM #2] - Use string interpolation inside tag content and the explicit previous correction requiring a single template literal. Instead of Sum of {a} and {b} is {a + b}, use one template literal like {Sum of ${a} and ${b} is ${a + b}} inside the <p> tag.
| // export const Sum = () => (); | ||
| export const Sum = ({ a = 0, b = 0 }) => ( | ||
| <p> | ||
| Sum of {a} and {b} is {a + b} |
There was a problem hiding this comment.
This violates checklist item #2: Use string interpolation inside tag content. Instead of separate Sum of {a} and {b} is {a + b} fragments, use a single template literal inside the <p> as required, e.g. <p>{Sum of ${a} and ${b} is ${a + b}}</p>.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved; the Sum component and App setup now match the described requirements and checklist items. The Sum component correctly uses a named export, defaults a and b to 0 when they’re missing, and renders the result inside a single interpolated string in a <p> element, covering [CHECKLIST ITEM #2]. In App.jsx, you included all five required usages of Sum (with different combinations of a and b), and your JSX formatting keeps short tags with fewer than three attributes on a single line, satisfying [CHECKLIST ITEM #1]. Functionally, the sums and text output are correct, and stylistically the components are clean and easy to read.
There are no code changes I’d flag as issues based on the current requirements, so you can be confident you’ve met the task goals. Nice work carefully incorporating the defaults, named exports, and interpolation details—these patterns will be useful in many future React components.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.