Skip to content

Solution#1792

Open
Kawka06 wants to merge 1 commit intomate-academy:masterfrom
Kawka06:develop
Open

Solution#1792
Kawka06 wants to merge 1 commit intomate-academy:masterfrom
Kawka06:develop

Conversation

@Kawka06
Copy link

@Kawka06 Kawka06 commented Jan 29, 2026

Copy link

@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

Great work on this task! Your application is fully functional and meets all the core requirements correctly. I am approving your solution. There is one minor point I'd like you to consider for future projects regarding [CHECKLIST ITEM #2]. The onClick handler for your AddButton uses an inline arrow function (onClick={() => handleSelect(good)}). While this works perfectly here, creating a new function on every render can lead to performance issues in larger applications. It's a great habit to define event handlers separately to avoid this. Keep up the excellent work!


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

Footnotes

  1. Rate AI review example

data-cy="AddButton"
type="button"
className="button"
onClick={() => handleSelect(good)}

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: 'Create separate methods instead of creating inline event handlers'. Using an inline arrow function here creates a new function for every AddButton on each re-render, which can affect performance in larger applications. While this is a common pattern for passing arguments, the checklist advises against it. Think about how you might handle this event without creating a new function inside the map callback.

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