Skip to content

task is completed#3935

Open
predator-bites wants to merge 3 commits into
mate-academy:masterfrom
predator-bites:develop
Open

task is completed#3935
predator-bites wants to merge 3 commits into
mate-academy:masterfrom
predator-bites:develop

Conversation

@predator-bites
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I’m requesting changes because there are critical checklist and requirement violations that affect task compliance. In src/App.jsx, two instances should omit a prop but currently pass explicit zeros: change b={0} to omit b for the a={10} case, and change a={0} to omit a for the b={5} case (these are required instances per the task description). Also in src/App.jsx, the tags with fewer than 3 short attributes are split across multiple lines; update them to single-line tags like <Sum a={2} b={3} /> and <Sum a={-5} b={5} /> to satisfy [CHECKLIST ITEM #1]. In src/components/Sum/Sum.jsx, use a single interpolated string inside the paragraph per [CHECKLIST ITEM #2], e.g., <p>{Sum of ${a} and ${b} is ${a + b}}</p>. These fixes are straightforward and will bring the solution in line with the functional requirements and style checklist. Nice work on using a named export and default values—once these adjustments are made, you should be all set.


✨ 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. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/App.jsx Outdated
<p>Sum of 0 and 0 is 0</p>
{/* Replace paragraphs with Sum componets */}
{/* And remove commented lines :) */}
<Sum
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #1: "[CODE STYLE] - If you have < 3 attribues on a tag and the values are short write the tag in one line (to make it easier to write and read)." The first has only two short attributes but is written across multiple lines. Consider writing it as a single-line tag: <Sum a={2} b={3} />. See the checklist for the exact rule.

Comment thread src/App.jsx Outdated
a = {2}
b = {3}
/>
<Sum
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #1: "[CODE STYLE] - If you have < 3 attribues on a tag and the values are short write the tag in one line (to make it easier to write and read)." The second has two short attributes but is written across multiple lines. Prefer the single-line form: <Sum a={-5} b={5} />.

Comment thread src/App.jsx Outdated
a = {-5}
b = {5}
/>
<Sum
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates the task description requirement that the App should include "just a = 10;" — the prop b must be omitted in that instance (so defaulting occurs). Currently b={0} is passed explicitly. Remove the explicit b={0} so the usage becomes <Sum a={10} />. Also this tag has < 3 short attributes and should be a single-line tag per checklist item #1: "[CODE STYLE] - If you have < 3 attribues on a tag and the values are short write the tag in one line (to make it easier to write and read)."

Comment thread src/App.jsx Outdated
a = {10}
b = {0}
/>
<Sum
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates the task description requirement that the App should include "just b = 5;" — the prop a must be omitted in that instance (so defaulting occurs). Currently a={0} is passed explicitly. Remove the explicit a={0} so the usage becomes <Sum b={5} />. Also this tag has < 3 short attributes and should be a single-line tag per checklist item #1: "[CODE STYLE] - If you have < 3 attribues on a tag and the values are short write the tag in one line (to make it easier to write and read)."

Comment thread src/components/Sum/Sum.jsx Outdated
@@ -1 +1,3 @@
// export const Sum = () => ();
export const Sum = ({ a = 0, b = 0 }) => (
<p>Sum of {a} and {b} is {a + b}</p>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #2: "2. [CODE STYLE] - Use string interpolation inside tag content". The paragraph currently uses multiple JSX expressions (Sum of {a} and {b} is {a + b}) instead of a single template string. Consider changing the content to use one interpolated string inside the paragraph, for example: <p>{Sum of ${a} and ${b} is ${a + b}}</p> so it matches the checklist's GOOD EXAMPLE.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I’m requesting changes because the Sum component violates a required checklist item and thus doesn’t fully meet the task criteria. Specifically, in src/components/Sum/Sum.jsx, the paragraph currently renders text by concatenating multiple JSX expressions (e.g., Sum of {a} and {b} is {a + b}), which breaks [CHECKLIST ITEM #2: Use string interpolation inside tag content]. This is a critical requirement, not a cosmetic preference.

Please update the paragraph to use a single template string, for example on the line rendering the <p> element: <p>{Sum of ${a} and ${b} is ${a + b}}</p>. This ensures the entire content is one interpolated string, matching the checklist’s good example.

Everything else looks good: the component uses default props, is a named export, and App renders all five required cases correctly. Once you make this small change, the solution should be ready to approve.


✨ 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. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread 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}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #2: "2. [CODE STYLE] - Use string interpolation inside tag content". The current paragraph builds the text by combining separate JSX pieces (Sum of {a} and {b} is {a + b}) instead of using one interpolated string. See the checklist for the exact requirement and the current file implementation .

Please update the paragraph to use a single template string so it matches the checklist's good example. For example:

export const Sum = ({ a = 0, b = 0 }) => (
  <p>{`Sum of ${a} and ${b} is ${a + b}`}</p>
);

This change will ensure the paragraph content is a single interpolated string as required.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Here is my approval: your Sum component is a named export with default parameters (a = 0, b = 0) and renders exactly one <p> using a single template string — great job following the requirements and style guidance. The App component correctly renders all five required cases: (2,3), (-5,5), (10), (b=5), and with no props, and keeps short JSX props on one line, which aligns with the style checklist.

Minor suggestions to strengthen your submission:

  • Consider adding a deployed demo link in your PR description and confirming tests pass (if applicable) so reviewers can verify behavior more easily.

Overall, the implementation is clean, concise, and meets the checklist. Nicely done—keep up the great work!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants