I've just created my first set of related classes in React and would like to get feedback from more experienced React developers.
I've used ES and completely vanilla React. The application is a shopping cart where the cart item quantities update the totals.
The one file situation is because it didn't seem to like multiple files, a build will fix this and that is not the scope of this question.
How does the application of BEM in a React context come across? What ways can I improve this, looking specifically at the application of React components, states and props?
I'm not specifically looking at any JavaScript only refactoring (such as those in the total calculation functions) nor at any HTML structure problems.
Pull and run from here - the CSS is also here.
var CartItem = React.createClass({
calculateTotal: function() {
return Number(this.props.price * this.props.qty).toFixed(2);
},
render: function() {
return (
<div key={this.props.key} className='cart-line'>
<div className='cart-line__name'>{this.props.name}</div>
<div className='cart-line__change-qty'>
<a className='cart-line__qty-up' onClick={this.props.onQtyChanged.bind(null, this.props.arrayIndex, '+')}>+</a>
<a className='cart-line__qty-down' onClick={this.props.onQtyChanged.bind(null, this.props.arrayIndex, '-')}>-</a>
</div>
<div className='cart-line__quantity'>{this.props.qty}</div>
<div className='cart-line__price'>£{Number(this.props.price).toFixed(2)}</div>
<div className='cart-line__total'>£{this.calculateTotal()}</div>
</div>
);
}
});
var AllCartItems = React.createClass({
buildRows: function() {
var rows = [];
var onQtyChanged = this.props.onQtyChanged;
var x = 0;
this.props.cartItems.forEach(function(cartItem) {
rows.push(<CartItem key={cartItem.id} arrayIndex={x} name={cartItem.name} qty={cartItem.qty} price={cartItem.price} onQtyChanged={onQtyChanged} />);
x ++;
});
return rows;
},
render: function() {
return (
<section className='all-cart-items'>
<h1 className='all-cart-items__heading'>Cart Items</h1>
<div className='all-cart-items__items'>
{this.buildRows()}
</div>
</section>
);
}
});
var Discount = React.createClass({
render: function() {
return (
<div className='discount'>
<input className='discount__field' type='text' name='discount' defaultValue='0.1'/>
<button className='discount__apply'>Apply Discount</button>
</div>
)
}
});
var TotalRow = React.createClass({
calculateTotal: function(total) {
return Number(total).toFixed(2);
},
render: function() {
return (
<div className='total-row'>
<span className='total-row__label'>{this.props.label}</span>
<span className='total-row__total'>£{this.calculateTotal(this.props.total)}</span>
</div>
);
}
});
var Totals = React.createClass({
discount: function() {
return 0 - this.total() * 0.1;
},
subTotal: function() {
var items = this.props.cartItems;
var subTotal = 0;
for (var x = 0; x < items.length; x ++) {
subTotal += items[x].price * items[x].qty;
}
return subTotal;
},
taxTotal: function() {
return this.subTotal() / 100 * 20;
},
total: function() {
return this.subTotal() + this.taxTotal();
},
youPay: function() {
return this.total() + this.discount()
},
render: function() {
return (
<section className='totals'>
<h1 className='totals__heading'>Totals</h1>
<div className='totals__details'>
<TotalRow label='Sub Total' total={this.subTotal()}/>
<TotalRow label='Discount Value' total=''/>
<TotalRow label='Tax @ 20%' total={this.taxTotal()}/>
<TotalRow label='Grand Total' total={this.total()}/>
<Discount />
<TotalRow label='Discount' total={this.discount()}/>
<TotalRow label='You Pay' total={this.youPay()}/>
</div>
</section>
);
}
});
var Basket = React.createClass({
getInitialState: function() {
return {cartItems};
},
handleQtyChanged: function(cartItemIndex, direction) {
if (direction === '+') {
cartItems[(cartItemIndex)].qty++;
} else {
cartItems[(cartItemIndex)].qty--;
}
this.setState({cartItems});
},
render: function() {
return (
<main>
<AllCartItems cartItems={this.state.cartItems} onQtyChanged={this.handleQtyChanged}/>
<Totals cartItems={this.state.cartItems} />
</main>
);
}
});
React.render(
<Basket cartItems={cartItems} />,
document.getElementById('react-basket')
);
1 Answer 1
First things First
I cloned it and run it as you wrote in README.md file, As a frontend project it'll be better if you used something like npm to run your project.
things you can do better:
When to use map over forEach?
If you want to return a new array instead of change the current, you can use map function. in this example you don't have to create a new array then change in it.
buildRows: function() {
var rows = [];
var onQtyChanged = this.props.onQtyChanged;
var x = 0;
this.props.cartItems.forEach(function(cartItem) {
rows.push(<CartItem key={cartItem.id} arrayIndex={x} name={cartItem.name} qty={cartItem.qty} price={cartItem.price} onQtyChanged={onQtyChanged} />);
x ++;
});
return rows;
},
You can simply do this
buildRows: function() {
return this.props.cartItems.map((cartItem, indx) =>
<CartItem
key={cartItem.id}
arrayIndex={indx}
name={cartItem.name}
qty={cartItem.qty}
price={cartItem.price}
onQtyChanged={this.props.onQtyChanged} />
);
},
You can use Object destruction
Object destruction is great for using object attributes without repeating the object name. you can use it in the previous example like this.
buildRows: function() {
const {onQtyChanged, cartItems} = this.props;
return cartItems.map(({id, name, qty, price}, indx) =>
<CartItem
key={id}
arrayIndex={indx}
name={name}
qty={qty}
price={price}
onQtyChanged={onQtyChanged}
/>
);
},
Use const and let instead of var
to avoid mutation or scope mistakes.
Ternary operator instead of if condition
Use ternary operators when you make just one line change like this
handleQtyChanged: function(cartItemIndex, direction) {
/* old
if (direction === '+') {
cartItems[(cartItemIndex)].qty++;
} else {
cartItems[(cartItemIndex)].qty--;
}
*/
// new
direction === '+'
? cartItems[(cartItemIndex)].qty++
: cartItems[(cartItemIndex)].qty--;
this.setState({cartItems});
},