Skip to content

Conversation

@maxeasy18
Copy link
Collaborator

Доделано прилодение из классной работы. Понадобилось много больше чем 4 часа.

@@ -0,0 +1,69 @@
import React, {Component} from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

the file should be named with hyphen instead of underscore

the file name should refer component it's exporting

admin-page.js -> class AdminPage

}

render() {
const items = this.props.items;
Copy link
Member

Choose a reason for hiding this comment

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

please use object destruction


render() {
const items = this.props.items;
const returnListOfItems = (items) => {
Copy link
Member

Choose a reason for hiding this comment

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

you could create a method instead of re-creating function every time component re-renders

const removeItem = () => {
this.props.removeItem(item.id);
}
return <AdminItem key={item.id} item={item} removeItem={removeItem} ></AdminItem>
Copy link
Member

Choose a reason for hiding this comment

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

if you not using children, please write component in short manner without closing tag


export class CartItem extends Component {
render() {
const item = this.props.item;
Copy link
Member

Choose a reason for hiding this comment

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

please use object destruction

const inCart = this.props.cart;
const returnListOfItems = (items) => {
return items.map((item ) => {
let inBasket = false;
Copy link
Member

Choose a reason for hiding this comment

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

const inBasket = inCart.includes(item.id)

if(inCart.indexOf(item.id) !== -1){
inBasket = true;
}
const addItemToCart = () => {
Copy link
Member

Choose a reason for hiding this comment

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

you have to move it to a method and make a closure to store item.id

const addItemToCart = () => {
this.props.addItemToCart(item.id);
}
return <ShopItem key={item.id} item={item} inBasket={inBasket} addItemToCart={addItemToCart} ></ShopItem>
Copy link
Member

Choose a reason for hiding this comment

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

please use shorthand Component notation

const addItemToCart = () => {
this.props.addItemToCart(item.id);
}
return <ShopItem key={item.id} item={item} inBasket={inBasket} addItemToCart={addItemToCart} ></ShopItem>
Copy link
Member

Choose a reason for hiding this comment

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

there a lot of properties it's better to make it multiline. Or just simply use a prettier

<div>
User
</div>
<Orders changePageToCartPage={this.props.changePageToCartPage} ></Orders>
Copy link
Member

Choose a reason for hiding this comment

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

shorthand notation for components
and extract all props via object destruction

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.

4 participants