Solution #4060
Solution #4060
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Good work on the Sum component's core logic. However, changes are needed because two key requirements were not met.
First, in src/App.jsx, you are explicitly passing 0 for props that should be omitted. The goal was to test the component's default prop functionality. Please adjust these calls to <Sum a={10} />, <Sum b={5} />, and <Sum /> as required by the task.
Second, the string rendering in src/components/Sum/Sum.jsx violates a checklist item [CHECKLIST ITEM #2]. It should be refactored to use a single template literal for better readability:
<p>{`Sum of ${a} and ${b} is ${a + b}`}</p>Please address these points to complete the task.
✨ 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
src/App.jsx
Outdated
| {/* 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 is to render a Sum component with just a = 10. You should omit the b prop and let the Sum component use its default value.
src/App.jsx
Outdated
| <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.
According to the task description, this Sum component should be rendered with just b = 5. The a prop should not be passed, allowing the default prop value to be used.
src/App.jsx
Outdated
| <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 Sum component should be rendered with 'no params at all'. Instead of explicitly passing 0 for a and b, you should remove the props entirely.
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} |
There was a problem hiding this comment.
This violates checklist item #2: 'Use string interpolation inside tag content'. While your code works, the good example in the checklist suggests using a single template literal for better readability, like {Sum of ${a} and ${b} is ${a + b}}.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on implementing the Sum component! You've correctly handled the props and default values within the component itself. However, a change is needed in src/App.jsx. The task requires you to test the default props by calling the component with no props at all. Your current implementation uses <Sum b={0} />, which doesn't fully test the default behavior for both props. To fix this, please update that line to be simply <Sum />. This will ensure all requirements are met.
✨ 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
| <Sum a={-5} b={5} /> | ||
| <Sum a={10} /> | ||
| <Sum b={5} /> | ||
| <Sum b={0} /> |
There was a problem hiding this comment.
The task requires a Sum component with 'no params at all' to test the default values for both props. This line should be <Sum /> instead.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving this submission as an educational override so you can move forward, but please take a moment to review this feedback.
Excellent work on the Sum component! It's implemented perfectly, and you correctly added the <Sum /> instance to test the default props. The only remaining issue is a small leftover in src/App.jsx. The component instance <Sum b={0} /> is no longer needed and should be removed, as the goal was to replace it with <Sum /> to test both default props simultaneously.
Great job addressing the previous feedback, and keep up the fantastic work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| <Sum a={-5} b={5} /> | ||
| <Sum a={10} /> | ||
| <Sum b={5} /> | ||
| <Sum b={0} /> |
There was a problem hiding this comment.
This component instance is not listed in the task requirements. The goal was to replace the line that previously tested only one default prop with <Sum /> to test both. You've added <Sum /> correctly on the next line, so this one can be removed.
2 participants
DEMO LINK