Assignment w2 react HANNA MELNYK #14
Conversation
hanna-melnyk
force-pushed
the
HANNA-MELNYK-w2-react
branch
from
ef39de2 to
d1b4d18
Compare
|
Please add link to deployed app in the PR description. |
hey, what do you mean by pr description?
|
I see it in the readme now. I missed it before. By PR description, I was referring to the first comment in the PR.
|
There was a problem hiding this comment.
Following was implemented
fake-datadirectory is removed and app connects to endpoints to fetch data ✅- API does the filtering ✅
- App shows error message when request failed ✅
Requested improvements:
- App shows loading while waiting for request to complete
- Use client side routing to route to product detail page
- Improve folder structure by moving components into a folder called
componentsif reusable andpagesif standalone. This structure improves maintainability.
| import {Product} from "./Product.jsx"; | ||
|
|
||
|
|
||
| export const AppRoutes = () => { |
There was a problem hiding this comment.
It's good practice to name the file the same name as the component. So either the file name can be changed to AppRoutes or component can be changed to Routes.
|
|
||
| export const CategoryList = () => { | ||
| const [categories, setCategories] = useState([]); | ||
| const [selectedCategory, setSelectedCategory] = useState(null); // Initialize state with null to show all products |
There was a problem hiding this comment.
Good use of comment to explain something that is not immediately clear from the code 👍
| <img src={product.image} alt={product.title}/> | ||
| <h1>{product.title}</h1> | ||
| <p>{product.price}</p> | ||
| <p>{product.description}</p> |
There was a problem hiding this comment.
Layout for this page can be improved to be more responsive.
Have a look at the live version
|
|
||
| if (loading) return <p>Loading...</p>; | ||
| if (error) return <p>Error: {error}</p>; | ||
| if (!product) return null; |
There was a problem hiding this comment.
Good practice to handle scenario where product is null 👍
This can be further improved by returning an informative message instead of null. E.g. Product was not found
| <Route path="/" element={<CategoryList />} /> | ||
| <Route path="/products" element={<ProductList />} /> | ||
| <Route path="/product/:id" element={<Product />} /> | ||
| </Routes> |
There was a problem hiding this comment.
Good job of setting up client side routing 👍
| } catch (error) { | ||
| setError(error.message); | ||
| } finally { | ||
| setLoading(false); |
| return ( | ||
| <div className="product-grid"> | ||
| {products.map((product) => ( | ||
| <div key={product.id} className="product"> |
There was a problem hiding this comment.
There isn't a link here which when clicked on will lead to the product details page.
No description provided.