Moayad Mohammed React Week 1 #2
Conversation
There was a problem hiding this comment.
Hi Moayad,
Great work on this first React assignment. I see you have a good grasp of React, rendering UI, and components. These are the core things we want you to learn about this week. Well done.
My comments are mostly about your CSS and HTML. It's very important not to overlook these two things, as they are the core foundations for any frontend development. Mastering them will make you an even better React developer.
Please review my comments and make those changes.
| } | ||
|
|
||
| h1 { | ||
| font-size: 3em; |
There was a problem hiding this comment.
In your css you use a combination of different units. rem, em, px, vh, etc. It's best not to use too many. I suggest you start by choosing between rem and em and sticking to just one of them. 🟢
There was a problem hiding this comment.
I have changed all of the fonts to rem to match it with the root element
| @@ -0,0 +1,16 @@ | |||
| import React from "react"; | |||
There was a problem hiding this comment.
The convention in React (and other frontend frameworks) is to name your component files in the same case as your component name - i.e. with a capital letter. Categories, Products, etc. 🟢
| {categories.map((category) => ( | ||
| <div key={category} | ||
| className = {`category ${category === selectedCategory ? 'category-selected' : ''}`}> | ||
| <h3 onClick={() => handleCategoryClick(category)}>{category}</h3> |
There was a problem hiding this comment.
It's important to use semantic HTML. Buttons should use the button element. https://www.freecodecamp.org/news/semantic-html-alternatives-to-using-divs/ 🟠
There was a problem hiding this comment.
replaced div with button element
week1/project/ecommerce/src/App.css
Outdated
| } | ||
|
|
||
| .product-img{ | ||
| width: 300px; |
There was a problem hiding this comment.
You've set a fixed height and width on the images. But not all images are the same width and height. This cause the images to become warped and stretch. Can you find a solution that doesn't utilise a fixed width and height? 🟠
There was a problem hiding this comment.
edit the product image to the following:
width: 100%; height: auto; object-fit: contain;
and added flex container for the container element
| @@ -0,0 +1,18 @@ | |||
| import React from "react"; | |||
|
|
|||
| const Products = ({ products }) => { | |||
website link:
https://66e16c8e606808e33b8e61f5--resilient-daifuku-620acc.netlify.app/