atividade Sum em React#3845
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Decision: REQUEST_CHANGES
Your project shows good structure and use of React, but there are a few critical blockers that prevent the requirements from being met and would likely cause failing tests.
Critical issues to fix
-
src/components/Sum/Sum.jsx
- Line 1 (component signature): The component currently accepts two positional arguments
Sum = (a = 0, b = 0). In React, components receive a singlepropsobject. Because of this,<Sum a={2} b={3} />won’t work as intended. Fix by destructuring from props with defaults, e.g.,export const Sum = ({ a = 0, b = 0 }) => { ... }. This also directly satisfies the defaulting requirement (checklist item 3.3). - Paragraph content: The sentence is split into multiple JSX expressions. The requirement (checklist item #2) is a single template literal expression:
<p>{Sum of ${a} and ${b} is ${a + b}}</p>.
- Line 1 (component signature): The component currently accepts two positional arguments
-
src/App.jsx
- Lines rendering Sum instances: The required five cases must be exactly:
<Sum a={2} b={3} />-
<Sum a={-5} b={5} />(currently reversed) -
<Sum a={10} />(omit b) -
<Sum b={5} />(omit a) -
<Sum />(omit both)
- Reason: This explicitly tests defaulting behavior. Providing zeros for missing props doesn’t validate defaults and violates the checklist/test cases.
- Lines rendering Sum instances: The required five cases must be exactly:
Minor improvement
- src/index.jsx line ~9: Remove the extraneous
<App />;line. It’s harmless but unnecessary.
You’re close—your file structure and named export are solid. Update the Sum signature and paragraph, and adjust the App test cases to match the specs. Once these are fixed, you should meet all requirements. Keep going!
✨ 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
| {/* Replace paragraphs with Sum componets */} | ||
| {/* And remove commented lines :) */} | ||
| <Sum a={2} b={3} /> | ||
| <Sum a={5} b={-5} /> |
There was a problem hiding this comment.
This violates checklist item #1: 'In App, where a tag has fewer than 3 attributes with short values (as with Sum a and b props), it should be written on a single line.' However, more importantly, the props passed do not match the required test cases. The second instance should be . Additionally, some instances should omit 'a' or 'b' to test defaulting behavior, e.g., , , and .
| {/* 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 checklist item #1 and the requirements: The third instance should be (b omitted), not . This is necessary to test defaulting of 'b' to 0.
| <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 checklist item #1 and the requirements: The fourth instance should be (a omitted), not . This is necessary to test defaulting of 'a' to 0.
| <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 checklist item #1 and the requirements: The fifth instance should be (both a and b omitted), not . This is necessary to test defaulting of both 'a' and 'b' to 0.
| @@ -1 +1,5 @@ | |||
| // export const Sum = () => (); | |||
| export const Sum = (a = 0, b = 0) => ( | |||
There was a problem hiding this comment.
This violates the core React functional component pattern. The component should accept a single props object, not two positional arguments. This also violates checklist item #3.3: 'Ensure defaulting of props a and b to 0 is implemented in code (e.g., via default parameter values, defaultProps, or nullish coalescing) so that absence of props results in numeric 0 values in both the displayed text and the computed sum.'
| export const Sum = (a = 0, b = 0) => ( | ||
| <p> | ||
| Sum of {a} and {b} is {a + b} | ||
| </p> |
There was a problem hiding this comment.
This violates checklist item #2: 'Construct the paragraph text using a single JSX expression with template literal interpolation, e.g.,
{Sum of ${a} and ${b} is ${a + b}}
|
|
||
| createRoot(document.getElementById('root')).render(<App />); | ||
|
|
||
| <App />; |
There was a problem hiding this comment.
The line <App />; is unnecessary and should be removed. It does not affect functionality but is extraneous and could cause confusion.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Decision: REQUEST_CHANGES
Your implementation is very close, but there’s one critical blocker related to an explicit checklist requirement.
Critical issue to fix (Checklist item #2):
- Location:
src/components/Sum/Sum.jsx, in thereturnstatement ofSum. - Issue: The paragraph content is split across multiple JSX expressions:
Sum of {a} and {b} is {a + b}. The requirement is to use a single template literal expression inside the paragraph. - Why this matters: The spec explicitly requires one interpolated string. Automated tests may check for this exact pattern and fail otherwise.
- How to fix: Replace the paragraph content with a single template literal, for example:
-
<p>{Sum of ${a} and ${b} is ${a + b}}</p>
-
What’s good:
Sumis a named export and correctly destructuresaandbwith defaults of0.App.jsxrenders exactly fiveSumcomponents with the required props and uses single-line JSX for short tags.
Please make the one-line change above, and you should be fully compliant. Nice work overall—this will be a quick fix!
✨ 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 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 the paragraph content to produce the final sentence in one expression, e.g., {Sum of ${a} and ${b} is ${a + b}}". The current implementation splits the sentence into multiple JSX expressions. Please use a single template literal expression for the paragraph content.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work overall—this is an educational approval to keep you moving forward while you learn. Your components are clean, well-structured, and you correctly used named exports and default values for a and b. One small fix is needed in src/components/Sum/Sum.jsx to meet checklist item #2: on the line rendering the paragraph (likely around the return), you currently have text split across multiple JSX expressions: Sum of {a} and {b} is {a + b}. This violates the requirement to use a single template literal. To fix it, replace the paragraph content with a single string interpolation: <p>{Sum of ${a} and ${b} is ${a + b}}</p>. This ensures the entire sentence is one expression, aligning with the checklist and avoiding potential automated check failures. Everything else looks solid—nice job handling the five Sum cases in App and keeping the JSX concise. Keep it up!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| @@ -1 +1,3 @@ | |||
| // export const Sum = () => (); | |||
| export const Sum = ({ a = 0, b = 0 }) => ( | |||
| <p>Sum of {a} and {b} is {a + b}</p> | |||
There was a problem hiding this comment.
This violates checklist item #2: "Inside the paragraph content, use string interpolation to construct the full sentence in one expression, e.g., {Sum of ${a} and ${b} is ${a + b}}, rather than concatenating multiple JSX text nodes and expressions." Please use a single template literal inside the JSX paragraph.
No description provided.