Skip to content

done - #4398

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

done#4398
sofiatsebenko wants to merge 3 commits into
mate-academy:masterfrom
sofiatsebenko:develop

Conversation

@sofiatsebenko

Copy link
Copy Markdown

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your implementation has critical violations of the checklist requirements that must be fixed. First, in App.jsx you're explicitly passing b={0} and a={0} to components that should only receive specific props - for example, 'just a = 10' means <Sum a={10} /> with NO b prop at all, and 'no params at all' means <Sum /> with no props passed (letting the component's defaults handle it). Second, in Sum.jsx you must use template literal syntax inside the tag content - change {a}, {b}, and {a + b} to `${a}`, `${b}`, and `${a + b}` respectively. These violations directly contradict [CHECKLIST ITEM #1] and [CHECKLIST ITEM #2].


✨ 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
{/* And remove commented lines :) */}
<Sum a={2} b={3} />
<Sum a={-5} b={5} />
<Sum a={10} b={0} />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checklist item #1 violation: When only passing a, don't explicitly set b={0}. The requirement states 'just a = 10' - meaning only a prop should be passed, not both. The Sum component handles missing b by defaulting to 0.

Comment thread src/App.jsx Outdated
<Sum a={2} b={3} />
<Sum a={-5} b={5} />
<Sum a={10} b={0} />
<Sum a={0} b={5} />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checklist item #1 violation: When only passing b, don't explicitly set a={0}. The requirement states 'just b = 5' - meaning only b prop should be passed, not both.

Comment thread src/App.jsx Outdated
<Sum a={-5} b={5} />
<Sum a={10} b={0} />
<Sum a={0} b={5} />
<Sum a={0} b={0} />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checklist item #1 violation: 'no params at all' means don't pass any props, not a={0} b={0}. Let the component use its default values.

Comment thread src/App.jsx
@@ -1,14 +1,13 @@
import React from 'react';
import './App.scss';
import { Sum } from './components/Sum/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.

Checklist item #2 violation: Must use template literal syntax for string interpolation inside tag content. Change {a} to `${a}`, {b} to `${b}`, and {a + b} to `${a + b}`.

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: 'Use string interpolation inside tag content'. The content should use template literal syntax: {Sum of ${a} and ${b} is ${a + b}} instead of spreading the string across multiple expressions.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your implementation has a critical violation of CHECKLIST ITEM #2 in src/components/Sum/Sum.jsx. The component currently uses separate template literals like {${a}}, {${b}}, and {${a + b}} instead of the required single template literal for the entire tag content. While the functionality works correctly, this violates the explicit checklist requirement which specifies using {`Sum of ${a} and ${b} is ${a + b}`} for better readability and consistency.

To fix this:

  • Open src/components/Sum/Sum.jsx
  • Change line 3 from multiple separate JSX expressions to a single template literal: {`Sum of ${a} and ${b} is ${a + b}`}

All other requirements pass (props with defaults, 5 Sum components, named export, correct output), so you just need to consolidate the template literal usage to meet the checklist standard.


✨ 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
@@ -1,14 +1,13 @@
import React from 'react';
import './App.scss';
import { Sum } from './components/Sum/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 #2] - Use string interpolation inside tag content. The GOOD example shows using a SINGLE template literal: {`Sum of ${a} and ${b} is ${a + b}`}. Currently there are multiple separate template literals which makes it harder to see the final result. Use one template literal that contains the entire string.

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. Use a single template literal inside the tag content instead of separate template literals for each value. Change to: {Sum of ${a} and ${b} is ${a + b}}

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your implementation is approved and fully meets all requirements. Both Sum and App components are correctly structured with named exports, default prop values, and proper string interpolation using template literals. You successfully implemented all 5 Sum components with the required prop combinations, and your code follows both style checklist items—attributes are written on single lines and template literals are used for the sum display. Excellent work on catching and fixing the previous checklist violation. Keep up the great coding practices!


✨ 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