Solution#4025
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 implementing the Sum component with default props. However, some changes are needed before this can be approved.
The primary issue is in src/App.jsx, where the Sum component is not being used as required to test the default props. Instead of omitting props, you are explicitly passing 0. To fix this, please update the component calls to <Sum a={10} />, <Sum b={5} />, and <Sum /> respectively.
Additionally, there are two code style violations:
- In
src/App.jsx, component calls with fewer than three attributes should be on a single line. [CHECKLIST ITEM #1] - In
src/components/Sum/Sum.jsx, the string should be constructed using a template literal, like{`Sum of ${a} and ${b} is ${a + b}`}. [CHECKLIST ITEM #2]
Please make these adjustments to meet all 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
| b = 0 | ||
| }) => ( | ||
| <p> | ||
| Sum of {a} and {b} is {a + b} |
There was a problem hiding this comment.
This doesn't follow the recommended code style. This violates checklist item #2: 'Use string interpolation inside tag content'. Please use a template literal to construct the string, like {`Sum of ${a} and ${b} is ${a + b}`}, for better readability.
| <Sum | ||
| a={2} | ||
| b={3} | ||
| /> |
There was a problem hiding this comment.
This violates checklist item #1: 'If you have < 3 attribues on a tag and the values are short write the tag in one line'. The Sum component here has only two attributes and should be written on a single line.
| <Sum | ||
| a={-5} | ||
| b={5} | ||
| /> |
There was a problem hiding this comment.
This also violates checklist item #1 regarding single-line tags for components with fewer than three attributes.
| <Sum | ||
| a={10} | ||
| b={0} | ||
| /> |
There was a problem hiding this comment.
The task requires a Sum component with 'just a = 10'. You should omit the b prop and let it use its default value. Also, this component usage violates checklist item #1 and should be on a single line.
| <Sum | ||
| a={0} | ||
| b={5} | ||
| /> |
There was a problem hiding this comment.
According to the task description, this Sum component should be called with 'just b = 5'. The a prop should be omitted to rely on its default value. This also violates the single-line style rule from checklist item #1.
| <Sum | ||
| a={0} | ||
| b={0} | ||
| /> |
There was a problem hiding this comment.
The requirements state that one Sum component should have 'no params at all'. Instead of passing 0 for both a and b, you should call the component without any props, like <Sum />, to use the default values. This also violates checklist item #1.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on implementing the Sum component and addressing the previous feedback! The component itself is perfect. However, changes are needed in src/App.jsx because the default props functionality is not being tested correctly. The goal is to demonstrate that the component defaults to 0 when a prop is omitted, but you are currently passing 0 explicitly.
Please update the last three Sum component calls to omit the props that should be defaulted:
- Change
<Sum a={10} b={0} />to<Sum a={10} /> - Change
<Sum a={0} b={5} />to<Sum b={5} /> - Change
<Sum a={0} b={0} />to<Sum />
Once you make these adjustments, your solution will fully meet the requirements. You're very close!
✨ 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={10} b={0} /> | ||
| <Sum a={0} b={5} /> | ||
| <Sum a={0} b={0} /> |
There was a problem hiding this comment.
The goal here is to test the default props functionality of the Sum component. Instead of explicitly passing 0, you should omit the props that should default to 0.
For example:
a = 10should be<Sum a={10} />b = 5should be<Sum b={5} />- no params should be
<Sum />
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I'm approving this solution as an educational override so you can move forward, but please take a moment to review this important feedback on default props.
Excellent work on the Sum component itself—it's implemented perfectly! You've also done a great job rendering most of the required cases in App.jsx.
The key area for improvement is how you're demonstrating the default prop functionality. To test if a default value is correctly applied, you must omit the prop entirely rather than passing an explicit value like 0. The correct way to render these components would be <Sum a={10} /> and <Sum b={5} />. This ensures that the component's internal default logic is actually being used.
Keep up the great effort!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
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.
The task requires rendering a Sum component with 'just a = 10'. To test the default prop functionality, the b prop should be omitted entirely here, like this: <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.
Similarly, for the case with 'just b = 5', the a prop should be omitted to let the default value take effect. The call should be <Sum b={5} />.
2 participants
DEMO LINK