Skip to content

product list eval pull request#153

Open
Rhardin378 wants to merge 13 commits intoprojectshft:masterfrom
Rhardin378:master
Open

product list eval pull request#153
Rhardin378 wants to merge 13 commits intoprojectshft:masterfrom
Rhardin378:master

Conversation

@Rhardin378
Copy link

No description provided.

import React from "react";
export const ProductList = () => {
//useEffect on page load make an api call that sets the products from our GET /products route

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beware of the app spacing

Comment on lines 23 to 25
for (let i = 0; i < count; i++) {
if (i % 9 == 0) {
if (i == 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code here is complex and not easy to understand and its not really maintainable. Consider extracting to different functions to help with readability.

return <ProductListItem key={product._id} product={product} />;
})}
</section>
<div className={styles.buttons}>{paginationButtons()}</div>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think pagination should have been its own component. Separating logic will make your code way more readable and clean.

Comment on lines +11 to +20
if (!category || category == "#") {
response = await axios.get("http://localhost:8001/products/", {
params: {
page: pageNum,
price: price,
query: query,
},
});
} else {
response = await axios.get("http://localhost:8001/products/", {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was really necessary to separate this?

return response.data;
}
);
export const productSlice = createSlice({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check spacing

return (
<>
<section className={styles.section}>
{products.map((product) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of map

<>
<section className={styles.section}>
{products.map((product) => {
return <ProductListItem key={product._id} product={product} />;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the separation of products and product.

import styles from "../page.module.css";
import { fetchProducts } from "../store/slices/product";
import axios from "axios";
export const SearchForm = () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not the best file name as this component is not only doing a search, but also handling the filters.

import styles from "../page.module.css";
import { fetchProducts } from "../store/slices/product";
import axios from "axios";
export const SearchForm = () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check your spacing! The whole file is hard to read because of the lack of proper spacing. Either too much, or not at all :(

// sets all options to a state variable
const getCategories = async () => {
try {
let response = await axios.get(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be const if you are not redefining it later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants