Conversation
Zibi95
left a comment
Zibi95
left a comment
There was a problem hiding this comment.
Hey. Well done project looks great. There are two things that i see missing.
- counter on the icons for cart and favourites
- swipe gesture on the pohones carousel on mobile. On the hero that event works.
What else:
- Fix data fetching — cache, deduplicate, add per-product API calls
-
Every page independently fetches the entire product catalog from all 3 categories:
- ProductPage.tsx - Calls Promise.all([getProductsByType('phones'), getProductsByType('tablets'), getProductsByType('accessories')]) just to display one product. Three full-catalog network requests for a single detail page.
- Favourites.tsx - Same pattern, loads everything to show a handful of items.
- HomePage.tsx - Also fetches all three categories.
- Cart.tsx - Calls getProductsGeneral() on every mount.
- No caching at all - No SWR, React Query, or even context-level cache. Navigating back and forth re-fetches everything.
Introduce a data layer (React Query/SWR or context-based cache). Product detail pages should have a getProductById endpoint, not load the entire DB.
2. Persist cart/favourites to localStorage
3. Add code splitting with React.lazy + Suspense
4. Memoize context values (useMemo)
5. Fix accessibility — ARIA attributes, focus management
6. Replace 404 error navigation with proper error states
7. Deduplicate SVGs into shared assets
8. Fix key={index} in Cart
9. Add tests for hooks, utilities, and key flows
10. Fix the copy-paste type name in FavouritesContext
| const [cart, setCart] = useState<string[]>([]); | ||
| const [favourites, setFavourites] = useState<string[]>([]); |
There was a problem hiding this comment.
Cart and Favourites state is plain useState<string[]>. All data is lost on page refresh. Must persist to localStorage and hydrate on mount.
| <CartContext.Provider value={{ cart, setCart }}> | ||
| <FavouritesContext.Provider value={{ favourites, setFavourites }}> |
There was a problem hiding this comment.
Context values are not memoized - every cart/favourites change re-renders all consumers (NavBar, every ProductCard, every page)
https://kentcdodds.com/blog/how-to-use-react-context-effectively
| const isSingle = el.count === 1; | ||
|
|
||
| return ( | ||
| <div key={index} className={styles.cartItem}> |
There was a problem hiding this comment.
Array Index as React Key. breaks reconciliation when items are removed.
| const cartToRender: { id: string; count: number }[] = []; | ||
|
|
||
| for (let i = 0; i < cart.length; i++) { | ||
| const existing = cartToRender.find(item => item.id === cart[i]); | ||
|
|
||
| if (existing) { | ||
| existing.count += 1; | ||
| } else { | ||
| cartToRender.push({ id: cart[i], count: 1 }); | ||
| } | ||
| } | ||
|
|
||
| cartToRender.sort((a, b) => a.id.localeCompare(b.id)); |
There was a problem hiding this comment.
Use a Map or reduce for O(n) with useMemo
| ), | ||
| ); | ||
| } catch { | ||
| navigate('*'); |
There was a problem hiding this comment.
A network error is not a "not found." Should show an error state UI.
| <Link to="https://github.com/Mykyta-snacj" className={styles.link}> | ||
| Rights | ||
| </Link> |
There was a problem hiding this comment.
React Router is for internal routes. External URLs need .
| { replace: true }, | ||
| ); | ||
| } | ||
| }, [navigate, searchParams]); |
There was a problem hiding this comment.
useEffect calls navigate() with searchParams in the dependency array. navigate changes searchParams, which re-triggers the effect. Only the params.length guard prevents an infinite loop - fires twice on every mount.
There was a problem hiding this comment.
95 models, 24 models, 100 models are hardcoded strings. Should come from the API
2 participants
No description provided.