-
Notifications
You must be signed in to change notification settings - Fork 64
React1 week2/soheib #48
base: main
Are you sure you want to change the base?
Conversation
<NavItem title={navbarItems[0].title} link={navbarItems[0].link} isActive={navbarItems[0].link === currentPath} /> | ||
<NavItem title={navbarItems[1].title} link={navbarItems[1].link} isActive={navbarItems[1].link === currentPath} /> | ||
<NavItem title={navbarItems[2].title} link={navbarItems[2].link} isActive={navbarItems[2].link === currentPath} /> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here instead of calling NavItem multiple times you could rely on map to make it cleaner and more modular :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh here I would separate the icons into different files and maybe store them inside an Icon folder to make it cleaner. Nice job using an SVG btw!
isSelected={isPlanetSelected} | ||
onAddOrRemovePlanet={onAddOrRemovePlanet} | ||
></PlanetCard> | ||
<PlanetCard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice opportunity for using map! If you store the information of the planets into an array of object you can make this more modular and clean!
Good job! |
Thank you in advance dear Mentor