Code review

First of, neat coding, fan of your works

I like how you're using propTypes and defaultProps, it always helps and makes the code more readable, and for a project the scale of yours, it's enough, but I'd rather see a common proptypes file where you assign your common types and reuse them everywhere, instead of copy-pasting types like the user for example, and modifying it on every instance whenever a change to the user object occurs.
Like this proptype:

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/Card/index.js#L321
----
https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/ButtonComponent/index.js#L40

Check out a library called `classnames`, makes the code a bit cleaner with things like this one

----

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/ButtonComponent/index.js#L5-L7

The fact that you're separately importing components from material ui is good, reduces the bundle size, wished that you did this in every other component.

----

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/Card/index.js#L23
https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/Card/index.js#L179

I'm not sure why would you use `makeStyles` and `useMediaQuery` when you've got better functionality in pure CSS, but either ways, you should have your styles in a separate file, so that you don't spam your files and fill them to 300 lines

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/Card/index.js#L178-L314

This entire section could've been split into two separate sections, this would increase the readability of your code.

----

Check out `i18n` for translation. A must have if your website is using an RTL layout or if it was multilingual

----

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/CardContainer/index.js#L39-L52

You must be able to write cleaner code than this, having everything mixed up together is spaghetti code.

You can either:
1. Separate your concerns, have your code split by functionality, as having your API calls in one place, your reducers in another, your components in another, your containers, your styles, ...etc. Having your code split by function helps you reusing every functionality.
2. Separate your components, have your code split by component, meaning that you'll have your component, and its container, and it's API calls, and its style, and everything required to run that component, all in one folder (NOT FILE), this would guarantee that adding or removing a component to or from the UI will not affect any other part of the UI.

But what you cannot do, is have everything that comes to mind all in one file, thrown to be run whenever called.

Check this article https://dev.to/surajjadhav/how-should-we-structure-our-react-code-1-2-1ecm

----

Consistency, you either use `useStyles`, or use pure `css`, but not both, you can name your css files `index.css`, `style.css` or `CardContainer.css`, but not a combination of them.

----

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/CardContainer/index.js#L128

LOL

----

Looking at your `Footer/index.js`, whenever any component of yours exceeds the 120 lines limit, this means you're doing something wrong. And whenever your footer exceeds the 20 lines limit, you're definitely doing something wrong.
Same for your header (700 lines!).

----

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/InputField/index.js#L41-L48

```js
className={clsx(
        classes[size],
        className
      )}
```
How about this? Try as much as possible not to nest ternary operators, I totally understand how easy the ternary is and how comforting it sounds, but it's not always readable.

---- 

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/component/index.js#L11

Why?

----

Instead of this:

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/pages/index.js#L1-L39

Check easier syntaxes here https://developer.mozilla.org/en-US/docs/web/javascript/reference/statements/export

---- 

looking at your utilities, no empty files, and you will definitely not have a file for each individual function you need in your application.

----

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/src/App.js#L79-L194

Just create an array of endpoints and map through that instead of repeating code.

----

https://github.com/GSG-G9/MASA-Store/blob/463acba3d6bf337101439f4fc57413196a250e4e/client/.env#L1

Someone has a problem running their application lol, upgrade or downgrade your Node version, that should solve your error.

---- 

Good work for starters, also for a project the size of a week, but you have to do much better in most other cases for a real job, every component you write is expected to be scalable and reusable everywhere, or else your employer will ask you to put a button somewhere your button isn't implemented to handle, and you'd have to rewrite it and modify every instance of that Button to fit the place.


	{!matches ? (
	<Card className={classes.root}>
	<Checkbox
	onChange={handleChange}
	name="checked"
	checked={checked}
	color="primary"
	value={data.product_id}
	/>
	<CardMedia className={classes.cover} image={data.img_url} />
	<div className={classes.cartContent}>
	<Typography component="h5" variant="h6">
	{data.name}
	</Typography>
	<Typography className={classes.ml20} variant="body1">
	{data.description}
	</Typography>
	</div>
	<div className={classes.details}>
	<CardContent className={classes.contentActions}>
	<Button>
	<Checkbox
	icon={<FavoriteBorder />}
	checkedIcon={<FavoriteIcon color="primary" />}
	checked={addedToFav}
	onChange={handelAddFav}
	name="checked"
	/>
	</Button>
	<Button onClick={handleDeleteFromCart}>
	<DeleteRoundedIcon color="primary" />
	</Button>
	</CardContent>
	<div className={classes.controls}>
	<Button
	variant="outlined"
	aria-label="increase"
	onClick={handleAddQuantity}
	className={classes.quantityButtonRight}
	>
	<AddIcon fontSize="small" />
	</Button>
	<TextField
	value={count}
	variant="outlined"
	className={classes.numberInput}
	/>
	<Button
	variant="outlined"
	aria-label="reduce"
	className={classes.quantityButtonLeft}
	onClick={handleSubQuantity}
	>
	<RemoveIcon fontSize="small" />
	</Button>
	</div>
	<div className={classes.priceLabel}>
	<Typography variant="h6" color="primary">
	{data.new_price}₪
	</Typography>
	</div>
	</div>
	</Card>
	) : (
	<Card className={classes.smallCard}>
	<CardHeader
	action={
	<Checkbox
	checked={checked}
	onChange={handleChange}
	name="checked"
	color="primary"
	/>
	}
	title={
	<div className={classes.headerContent}>
	<Typography variant="h6">{data.name}</Typography>
	<CardActions>
	<Button>
	<Checkbox
	icon={<FavoriteBorder />}
	checkedIcon={<FavoriteIcon color="primary" />}
	checked={addedToFav}
	onChange={handelAddFav}
	name="checked"
	/>
	</Button>
	<Button onClick={handleDeleteFromCart}>
	<DeleteRoundedIcon color="primary" />
	</Button>
	</CardActions>
	</div>
	}
	/>

	<CardMedia className={classes.media} image={data.img_url} />
	<CardContent>
	<div className={classes.controls}>
	<Typography className={classes.ml20} variant="h6">
	{data.description}
	</Typography>
	</div>
	</CardContent>

	<div className={classes.controls}>
	<Button
	variant="outlined"
	aria-label="reduce"
	className={classes.quantityButtonRight}
	onClick={() => {
	setCount(Math.max(count - 1, 1));
	}}
	>
	<RemoveIcon fontSize="small" />
	</Button>
	<TextField
	value={count}
	variant="outlined"
	id="outlined-basic"
	className={classes.numberInput}
	/>
	<Button
	variant="outlined"
	aria-label="increase"
	className={classes.quantityButtonLeft}
	onClick={() => {
	setCount(count + 1);
	}}
	>
	<AddIcon fontSize="small" />
	</Button>
	</div>
	<div className={classes.priceLabel}>
	<Typography variant="h6">{data.new_price} ₪ </Typography>
	</div>
	</Card>
	)}

	const addFavorite = async (id) => {
	try {
	await axios({
	method: 'post',
	url: `/api/v1/favorite/${userData.profileData.id}`,
	headers: {},
	data: {
	productId: id,
	},
	});
	} catch (error) {
	setOpen(true);
	}
	};

	className={clsx(
	size === 'small'
	? classes.small
	: size === 'medium'
	? classes.medium
	: classes.large,
	className
	)}

	import HomePage from './Common/HomePage';
	import AddProductsAdminPage from './Admin/AddProductsAdminPage';
	import CategoryProductPage from './Common/CategoryProductPage';
	import SearchPage from './Common/SearchPage';
	import FavoritePage from './User/FavoritePage';
	import ProductsAdminPage from './Admin/ProductsAdminPage';
	import ProfilePage from './User/ProfilePage';
	import SignUpPage from './Common/SignUpPage';
	import SignInPage from './Common/SignInPage';
	import PaymentPage from './User/paymentPage';
	import OrderPage from './User/OrderPage';
	import CartPage from './User/CartPage';
	import AdminHomePage from './Admin/AdminHomePage';
	import OrdersPage from './Admin/OrdersPage';
	import ClientsPage from './Admin/ClientsPage';
	import EditProductsAdminPage from './Admin/EditProductsAdminPage';
	import NotFoundPage from './Common/404';
	import ProductDetailsPage from './Common/ProductDetailsPage';

	export {
	HomePage,
	AddProductsAdminPage,
	CategoryProductPage,
	SearchPage,
	FavoritePage,
	ProductsAdminPage,
	ProfilePage,
	SignInPage,
	SignUpPage,
	PaymentPage,
	OrderPage,
	CartPage,
	AdminHomePage,
	OrdersPage,
	ClientsPage,
	EditProductsAdminPage,
	ProductDetailsPage,
	NotFoundPage,
	};

	switch (type) {
	case 'user':
	return (
	<StylesProvider jss={jss}>
	<ThemeProvider theme={theme}>
	<Header type="user" userData={userData} setType={setType} />
	<Switch>
	<Route exact path="/">
	<HomePage type="user" userData={userData} />
	</Route>
	<Route exact path="/products/:category">
	<CategoryProductPage type="user" userData={userData} />
	</Route>
	<Route exact path="/product/:productId">
	<ProductDetailsPage type="user" userData={userData} />
	</Route>
	<Route exact path="/search">
	<SearchPage type="user" userData={userData} />
	</Route>
	<Route exact path="/profile">
	<ProfilePage type="user" userData={userData} />
	</Route>
	<Route exact path="/favorite">
	<FavoritePage type="user" userData={userData} />
	</Route>
	<Route exact path="/payment">
	<PaymentPage type="user" userData={userData} />
	</Route>
	<Route exact path="/order">
	<OrderPage type="user" userData={userData} />
	</Route>
	<Route exact path="/cart">
	<CartPage type="user" setType={setType} userData={userData} />
	</Route>
	<Route>
	<NotFoundPage type="user" />
	</Route>
	</Switch>
	<Footer />
	</ThemeProvider>
	</StylesProvider>
	);

	case 'admin':
	return (
	<StylesProvider jss={jss}>
	<ThemeProvider theme={theme}>
	<Header type="admin" userData={userData} setType={setType} />
	<Switch>
	<Route exact path="/admin">
	<AdminHomePage type="admin" />
	</Route>
	<Route exact path="/admin/products">
	<ProductsAdminPage type="admin" />
	</Route>
	<Route exact path="/admin/add-product/:productId">
	<AddProductsAdminPage type="admin" />
	</Route>
	<Route exact path="/admin/orders">
	<OrdersPage type="admin" />
	</Route>
	<Route exact path="/admin/clients">
	<ClientsPage type="admin" />
	</Route>
	<Route exact path="/admin/edit-product/:productId">
	<EditProductsAdminPage type="admin" />
	</Route>
	<Route>
	<NotFoundPage type="admin" />
	</Route>
	</Switch>
	<Footer type="admin" />
	</ThemeProvider>
	</StylesProvider>
	);
	default:
	return (
	<StylesProvider jss={jss}>
	<ThemeProvider theme={theme}>
	<Switch>
	<Route exact path="/">
	<Header type="guest" userData={userData} setType={setType} />
	<HomePage type="guest" userData={userData} />
	<Footer />
	</Route>
	<Route exact path="/products/:category">
	<Header type="guest" userData={userData} setType={setType} />
	<CategoryProductPage type="guest" userData={userData} />
	<Footer />
	</Route>
	<Route exact path="/search">
	<Header type="guest" userData={userData} setType={setType} />
	<SearchPage type="guest" userData={userData} />
	<Footer />
	</Route>
	<Route exact path="/sign-in">
	<SignInPage setType={setType} />
	</Route>
	<Route exact path="/sign-up">
	<SignUpPage setType={setType} />
	</Route>
	<Route exact path="/product/:productId">
	<Header type="guest" userData={userData} setType={setType} />
	<ProductDetailsPage type="guest" userData={userData} />
	<Footer />
	</Route>
	<Route>
	<Header type="guest" userData={userData} setType={setType} />
	<NotFoundPage type="guest" />
	<Footer />
	</Route>
	</Switch>
	</ThemeProvider>
	</StylesProvider>
	);
	}

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code review #130

MASA-Store/client/src/component/Card/index.js

Line 321 in 463acba

data: PropTypes.object.isRequired,

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	import Button from '@material-ui/core/Button';
	import FavoriteBorderIcon from '@material-ui/icons/FavoriteBorder';
	import FavoriteIcon from '@material-ui/icons/Favorite';

Code review #130

Description

MASA-Store/client/src/component/Card/index.js Line 321 in 463acba data: PropTypes.object.isRequired,

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

MASA-Store/client/src/component/Card/index.js

Line 321 in 463acba

data: PropTypes.object.isRequired,