Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deliver Eat Lemony #23

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

LemonyDesign
Copy link

Tweaks made to ensure still working as intended for now.
Functionality - error handling for addition of products.
Needs work on shopping basket and control of component appearance
Also on design - but this at mobile MVP as part of current development

this.setState({currentOrder: updatedOrder})
}
removeItemOrder(id) {
const array = [...this.state.currentOrder];
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be easier to store orders in an object that uses ids as keys as it would make look ups much simpler

// values = Object.values(this.props.menu);
// values.map();
return this.props.menu.filter(menuitem => menuitem.category === course)
.map(menuitem => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation here would make code easier to read


const buttonclasses = cx('menuitem__submit', {
'added': this.state.added,
'': !this.state.added
Copy link
Contributor

Choose a reason for hiding this comment

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

think this can be removed as it does not add any classes

<ul className="menu--settings">
{Object.keys(this.props.orders).map(order => {
console.log({order})
return <OrderAdminItem
Copy link
Contributor

Choose a reason for hiding this comment

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

OrderAdminItem will need a key

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