Fixed all issues( #1 to #8)#68
Fixed all issues( #1 to #8)#68brudevtek wants to merge 8 commits intodeveloper-job-simulation:mainfrom
Conversation
… tests - attempt2
| filterOptions, | ||
| setFilterOptions, | ||
| sortOptions, | ||
| setSortOptions, | ||
| products, |
There was a problem hiding this comment.
suggestion: This is quite a lot of props coming in to this component. This usually indicates a smell. Check out how I fixed the functionality of this component here, without needing to add many props.
src/Components/ProductFilters.jsx
Outdated
| filterOptions.price.forEach((element) => { | ||
| if (element.minValue.toString() === e.target.value) { | ||
| if (element.checked === false) { | ||
| element.checked = true; | ||
| } else if (element.checked === true) | ||
| element.checked = false; | ||
| } | ||
| }); | ||
| let selected_array = []; | ||
|
|
||
| let selected_products = filterOptions.price.filter( | ||
| function (x) { | ||
| return x.checked === true; | ||
| } | ||
| ); | ||
|
|
||
| selected_products.forEach((element) => { | ||
| selected_array.push( | ||
| allproducts.filter(function (item) { | ||
| return ( | ||
| item.price > element.minValue && | ||
| item.price < element.maxValue | ||
| ); | ||
| }) | ||
| ); | ||
| }); | ||
| if (selected_array.length > 0) { | ||
| setProducts(selected_array.flat(1)); | ||
| setselectedByPrice(selected_array.flat(1)); | ||
| } else { | ||
| if (selectedByColor.length > 0) { | ||
| setProducts(selectedByColor); | ||
| } else { | ||
| setProducts(allproducts); | ||
| setselectedByPrice(allproducts); | ||
| } | ||
| } | ||
|
|
||
| set_Nbr_Filters_price(selected_array.length); |
There was a problem hiding this comment.
suggestion: We should try and refactor this. It's quite a large chunk of code which is hard to maneuver at first glance.
I also notice its very similar to some of the code below (160 - 206). Given it's size right, it helps to isolate this to a single helper function, and then call it from the onClick handlers.
sbmsr
left a comment
sbmsr
left a comment
There was a problem hiding this comment.
All in all, nicely done! I see the exercise definitely got the gears turning. I would take a peek at my PR for some alternative approaches to fixing the issues in the project.
I've added lots of constructive feedback. Let me know what you think.
2 participants
Hey, could you please review my changes. I fixed issues and added the new features as requested in issues#1 to 8. My previous PR failed the automated tests. This is a new one after attempting to resolve the issue.
Happy to discuss the changes if necessary.