ilias-khugaev-week3-react #26
Conversation
crevulus
left a comment
crevulus
left a comment
There was a problem hiding this comment.
Hi Ilias,
Once again you've left things out of the project that are requirements from the README! Please make sure your work is complete before you submit it. This will be important in your career as a developer when you're fixing bugs and delivering features.
| if (!product) return <div>Product not found</div>; | ||
|
|
||
| return ( | ||
| <> |
There was a problem hiding this comment.
You are missing a requirement from the README:
For the images in the product list as well as the product details page there needs to be a heart that if clicked will update the favourites array in the context.
🔴
| const [loading, setLoading] = useState(false); | ||
| const [error, setError] = useState(null); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
You should be using your useFetch function here 🔴
| .then(data => setProduct(data)) | ||
| .catch(err => { | ||
| console.error("Error when receiving the product:", err); | ||
| setError("Couldn't upload the product"); |
There was a problem hiding this comment.
the user isn's trying to upload anything - could you make this message a little more accurate? 🟠
|
|
||
| function Products({ products = [] }) { | ||
|
|
||
| if (!products.length) { |
| : (allItems || []); | ||
|
|
||
| if (error) { | ||
| return <div className="error">Data upload error: {error.message || String(error)}</div>; |
| <NavBar /> | ||
| </div> | ||
|
|
||
| {loading ? ( |
There was a problem hiding this comment.
This is a nested ternary and is not a good idea for readability and maintainability reasons: https://dev.to/junihoj/why-nested-ternary-operators-are-bad-practice-a-guide-for-developers-ki1 🟠
| import { useContext } from 'react'; | ||
| import { FavoritesContext } from '../Context/FavoritesContext'; | ||
|
|
||
| const FavIcon = ({ productId }) => { |
There was a problem hiding this comment.
Be careful with naming your components. a "favicon" is already a thing - it's the icon that shows in your browser tab. Plus, this component doesn't render an icon. The icon(s) are in svg files in your assts folder. This file renders a button that users can interact with, so something like FavButton would be better. 🔵
| }; | ||
|
|
||
| return ( | ||
| <img |
There was a problem hiding this comment.
It's important to use semantic HTML. Clickable elements should be buttons or anchor tags. This article is about divs but it applies to many elements: https://dev.to/kenbellows/stop-using-so-many-divs-an-intro-to-semantic-html-3i9i 🟠
|
@crevulus Thank you so much! I made adjustments based on your comments. |
2 participants
ilias-khugaev-week3-react.netlify.app