Wishlist menu

[yashseth391]

I have made the feature and tested it locally as well , @Alexandrbig1 , please review the pr and let me know for any changes.

[yashseth391]

@Alexandrbig1 please review it.

[Alexandrbig1]

@yashseth391, please solve conflicts and update PR.

[yashseth391]

@Alexandrbig1 , i have resolved the conflicts , please review it.

[Alexandrbig1]

@yashseth391, thanks for your updates. Please make few improvements, but before fetch latest updates, and test before next PR:

• Missing icon import (BLOCKER)
• Impact: Runtime crash when opening wishlist (X used but not imported).
• Fix: Add X to lucide-react imports in MainHeader.jsx (and ensure Heart/Search are imported). Example: import { User, ShoppingCart, Heart, Search, X } from 'lucide-react';
• Unguarded product fields -> runtime exceptions (HIGH)
• Impact: price.toFixed or destructuring product when product is null/undefined will crash the UI.
• Fix: Guard product and numeric fields before rendering. Example: if (!product) return null; const priceStr = typeof price === 'number' ? price.toFixed(2) : '—';
• Reducer correctness: duplicates & loose equality (HIGH)
• Impact: Duplicate wishlist entries; remove may fail when id types differ (string vs number).
• Fix: Prevent duplicates and use strict comparison. Example addToWishList: if (!state.items.some(i => i.id === payload.id)) state.items.push(payload); Example RemoveFromWishList: state.items = state.items.filter(i => i.id !== payload.id);
• Inconsistent/unused prop and action usage (MED-HIGH)
• Impact: Confusing API and maintainability issues (MainHeader passes RemoveFromWishList prop but child imports and dispatches it).
• Fix: Either pass an onRemove callback from the parent (preferred) or stop passing the action prop and let child import actions. Keep one pattern.
• Accessibility and keyboard handling for the drawer (MED-HIGH)
• Impact: Keyboard/screen-reader users can’t reliably interact; UX regression.
• Fix: Add role="dialog", aria-modal="true", focus trap, and Escape key handler to close the drawer. Ensure the close button is focusable.
• Untrusted image URLs / resource handling (MED)
• Impact: Privacy/mixed-content/availability issues and possible security/UX problems if imageUrl is untrusted or missing.
• Fix: Validate/whitelist image URLs or use a safe image proxy/placeholder and add onError handler.

Quick acceptance guidance

• Block merge until (1) missing icon import and (2) unguarded product/price issues are fixed.
• Fix reducer duplicate/strict-equality issues and at least add Escape/focus handling before merging if you require good accessibility quality on merge; otherwise require these as follow-up tasks assigned to the author.

[yashseth391]

@yashseth391, thanks for your updates. Please make few improvements, but before fetch latest updates, and test before next PR:

• Missing icon import (BLOCKER)
• Impact: Runtime crash when opening wishlist (X used but not imported).
• Fix: Add X to lucide-react imports in MainHeader.jsx (and ensure Heart/Search are imported). Example: import { User, ShoppingCart, Heart, Search, X } from 'lucide-react';
• Unguarded product fields -> runtime exceptions (HIGH)
• Impact: price.toFixed or destructuring product when product is null/undefined will crash the UI.
• Fix: Guard product and numeric fields before rendering. Example: if (!product) return null; const priceStr = typeof price === 'number' ? price.toFixed(2) : '—';
• Reducer correctness: duplicates & loose equality (HIGH)
• Impact: Duplicate wishlist entries; remove may fail when id types differ (string vs number).
• Fix: Prevent duplicates and use strict comparison. Example addToWishList: if (!state.items.some(i => i.id === payload.id)) state.items.push(payload); Example RemoveFromWishList: state.items = state.items.filter(i => i.id !== payload.id);
• Inconsistent/unused prop and action usage (MED-HIGH)
• Impact: Confusing API and maintainability issues (MainHeader passes RemoveFromWishList prop but child imports and dispatches it).
• Fix: Either pass an onRemove callback from the parent (preferred) or stop passing the action prop and let child import actions. Keep one pattern.
• Accessibility and keyboard handling for the drawer (MED-HIGH)
• Impact: Keyboard/screen-reader users can’t reliably interact; UX regression.
• Fix: Add role="dialog", aria-modal="true", focus trap, and Escape key handler to close the drawer. Ensure the close button is focusable.
• Untrusted image URLs / resource handling (MED)
• Impact: Privacy/mixed-content/availability issues and possible security/UX problems if imageUrl is untrusted or missing.
• Fix: Validate/whitelist image URLs or use a safe image proxy/placeholder and add onError handler.

Quick acceptance guidance

• Block merge until (1) missing icon import and (2) unguarded product/price issues are fixed.
• Fix reducer duplicate/strict-equality issues and at least add Escape/focus handling before merging if you require good accessibility quality on merge; otherwise require these as follow-up tasks assigned to the author.

@Alexandrbig1 thank you for the update, i will make the changes.

[yashseth391]

@Alexandrbig1 , thank you for the updates of the last pr. I learnt a lot after fixing these , I have fixed all these , please review it . Also , some changes were already updated . Let me know if this pr requires further fixes.

[Alexandrbig1]

@yashseth391, looks good overall — but please address a few important issues before merging:

• Create a separate component for WishList, do not store it in the Products folder.
• Check the design, check once again description of the task, test locally before PR.
• MainHeader.jsx imports useSelector twice — remove the duplicate import.
• ProductCard state: it dispatches add/remove but no longer derives isWishlisted from the store — use useSelector (or keep a prop and update parent) so UI toggles reliably.
• Filename/typo: WishiListScreen.jsx spelling is inconsistent — consider renaming to WishListScreen and keep export names consistent.
• imageUrl is used directly in . Validate/sanitize URLs (or restrict schemes) to reduce risk from malicious data URIs / SVGs. Use default image as already in Product card.
• Storing full product objects in Redux is okay for now, but avoid persisting sensitive fields.
• Use consistent action naming (removeFromWishList vs RemoveFromWishList).

Thank you

[yashseth391]

@Alexandrbig1 , Is the pr good now ?

[Alexandrbig1]

@yashseth391, thanks for the PR! A few required fixes before I can approve:

• Rename files/components: fix the typo "WishiList…" → "WishList…" (rename WishiListScreen / WishiListDrawerScreen) and keep a single WishList component under src/components/WishList. Remove duplicate WishListProduct in src/components/Products.
• Unify action/utility names and casing (use camelCase): e.g. addToWishlist / removeFromWishlist across wishListSlice, utils, and all imports.
• ProductCard must derive wishlist state from Redux (useSelector) or be updated by the parent on change. Right now it dispatches add/remove but still uses the initial prop, so the UI can get out of sync.
• MainHeader: remove duplicate imports and ensure useSelector/useDispatch are imported once; verify SearchBox is correctly imported/used.
• Image handling: don’t render unvalidated data: URIs / SVGs directly. Add a fallback to the default placeholder to avoid XSS/SVG risks.

Before creating a new PR, make sure you fetch the latest changes, test your implementation locally, and run npm run lint to clean all errors.

[yashseth391]

@yashseth391, thanks for the PR! A few required fixes before I can approve:

• Rename files/components: fix the typo "WishiList…" → "WishList…" (rename WishiListScreen / WishiListDrawerScreen) and keep a single WishList component under src/components/WishList. Remove duplicate WishListProduct in src/components/Products.
• Unify action/utility names and casing (use camelCase): e.g. addToWishlist / removeFromWishlist across wishListSlice, utils, and all imports.
• ProductCard must derive wishlist state from Redux (useSelector) or be updated by the parent on change. Right now it dispatches add/remove but still uses the initial prop, so the UI can get out of sync.
• MainHeader: remove duplicate imports and ensure useSelector/useDispatch are imported once; verify SearchBox is correctly imported/used.
• Image handling: don’t render unvalidated data: URIs / SVGs directly. Add a fallback to the default placeholder to avoid XSS/SVG risks.

Before creating a new PR, make sure you fetch the latest changes, test your implementation locally, and run npm run lint to clean all errors.

@Alexandrbig1
For the product card, which i have used in wishlist , i have used useSelector, i have not changed searchBox code for the image url fallback , used the same one which is already used in all products page.

For the camel case and renaming and duplication , i have deleted. Please review it .

For the other changes , can i raise a new pr , since those are not related to wishListMenu . I think creating a new file , fallBack list will help much , which will hold all the fallBack array , images all data , by using where ever we render it we can use nullish operator and pass the variable .If the idea seems good , I will raise a new pr for it.

[yashseth391]

@Alexandrbig1 ,please review it.

[Alexandrbig1]

@yashseth391, have you test it? There not even imports of useSelector and useDispatch in MainHeader component, it'll definetely show an error. Please, test it locally, run npm run lint` and if everything is works and ready, you can open PR, but if this section is difficult, let me know, maybe someone else would work on it. This is very important section.

Thanks for the PR — a few blockers before I can approve:

• MainHeader.jsx uses useSelector/useDispatch but doesn’t import them. Add: import { useSelector, useDispatch } from 'react-redux';
• utils/wishlist.js renamed removeFromWishlist → RemoveFromWishlist. Revert to removeFromWishlist (lowercase) or update all callers — this breaks existing API.
• ProductCard dispatches wishlist actions but still takes an isWishlisted prop (initial). This can leave the UI out-of-sync. Either read wishlist from Redux in the component or restore a parent onToggle callback.
• File name typo: src/components/WishList/WishiListScreen.jsx — rename to a consistent name (e.g., WishListDrawerScreen.jsx) to avoid case-sensitive FS issues.

Please apply these fixes and I’ll

[yashseth391]

@Alexandrbig1 , These changes i have already done , but sorry , i looked those were on my local those were not pushed , I have pushed them .Also there is some function which is not used in mainHeader.jsx , but its not used anymore , some other person was working on it , should i clean that as well .