Develop#4284
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has two critical issues that need fixing before approval. First, the Sum component violates [CHECKLIST ITEM #2] by using separate JSX expressions {a}, {b}, {a + b} instead of the required template literal string interpolation. Replace with {Sum of ${a} and ${b} is ${a + b}}. Second, the App component incorrectly passes explicit 0 values for the last three Sum instances: line 9 passes b={0} when it should omit b entirely, line 10 passes a={0} when it should omit a, and line 11 passes both a={0} b={0} when it should pass no props at all. The requirements specifically state 'just a = 10', 'just b = 5', and 'no params at all' — the Sum component already defaults missing props to 0, so omit them entirely as specified.
✨ 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
| export const App = () => ( | ||
| <> |
There was a problem hiding this comment.
This violates checklist item #2 which requires using string interpolation with template literals. Replace with: {Sum of ${a} and ${b} is ${a + b}}
| {/* And remove commented lines :) */} | ||
| <Sum a={2} b={3} /> | ||
| <Sum a={-5} b={5} /> | ||
| <Sum a={10} b={0} /> |
There was a problem hiding this comment.
The requirement says 'just a = 10' - do not pass b at all. The Sum component already defaults b to 0 when not provided. Change to <Sum a={10} />
| <Sum a={2} b={3} /> | ||
| <Sum a={-5} b={5} /> | ||
| <Sum a={10} b={0} /> | ||
| <Sum a={0} b={5} /> |
There was a problem hiding this comment.
The requirement says 'just b = 5' - do not pass a at all. The Sum component already defaults a to 0 when not provided. Change to <Sum b={5} />
| <Sum a={-5} b={5} /> | ||
| <Sum a={10} b={0} /> | ||
| <Sum a={0} b={5} /> | ||
| <Sum a={0} b={0} /> |
There was a problem hiding this comment.
The requirement says 'no params at all' - do not pass any props. The Sum component already defaults both values to 0. Change to <Sum />
| <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. Replace with template literal: {Sum of ${a} and ${b} is ${a + b}}
| Sum of {a} and {b} is {a + b} | ||
| </p> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
The requirement says 'just a = 10' (no b prop). Since Sum defaults b to 0, use <Sum a={10} />
| </p> | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
The requirement says 'just b = 5' (no a prop). Since Sum defaults a to 0, use <Sum b={5} />
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your Sum component implementation is correct and meets the functional requirements. However, the App component has critical issues with the last three Sum instances where explicit 0 values are being passed instead of omitting those props entirely as specified in the requirements:
- For "just
a = 10" — change<Sum a={10} b={0} />to<Sum a={10} /> - For "just
b = 5" — change<Sum a={0} b={5} />to<Sum b={5} /> - For "no params at all" — change
<Sum a={0} b={0} />to<Sum />
The Sum component already defaults both a and b to 0 in its destructuring, so passing 0 explicitly is redundant and violates the explicit specification that these props should be omitted. The requirements clearly distinguish between "just a = 10" (which means only a) and passing both props. Once you remove those explicit 0 values, your implementation will be complete and correct.
✨ 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
| {/* And remove commented lines :) */} | ||
| <Sum a={2} b={3} /> | ||
| <Sum a={-5} b={5} /> | ||
| <Sum a={10} b={0} /> |
There was a problem hiding this comment.
This violates the requirements. For 'just a = 10', the b prop should be omitted entirely, not set to 0. The Sum component already defaults b to 0, so pass only a.
| <Sum a={2} b={3} /> | ||
| <Sum a={-5} b={5} /> | ||
| <Sum a={10} b={0} /> | ||
| <Sum a={0} b={5} /> |
There was a problem hiding this comment.
This violates the requirements. For 'just b = 5', the a prop should be omitted entirely, not set to 0. The Sum component already defaults a to 0, so pass only b.
| <Sum a={-5} b={5} /> | ||
| <Sum a={10} b={0} /> | ||
| <Sum a={0} b={5} /> | ||
| <Sum a={0} b={0} /> |
There was a problem hiding this comment.
This violates the requirements. For 'no params at all', no props should be passed to this Sum component. Rely on the component's internal defaults for both a and b.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation is approved and meets all requirements perfectly. The Sum component correctly accepts a and b props with default values of 0, uses a named export, and renders the sum using template literals as specified. All five Sum instances in the App component have the correct prop combinations, and the previous issues with explicit 0 values have been resolved. Your code follows the style checklist guidelines, including one-line tags for multiple attributes and proper template literal interpolation. Great work on this assignment!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
3 participants
The
Appshould contain the nextSumcomponents:a = 2andb = 3;a = -5andb = 5;a = 10;b = 5;Instructions
npm testto ensure your solution is correct.<your_account>with your Github username in the DEMO LINK and add it to the PR description.