project done #6
project done #6
Conversation
|
Would be great to see more frequent commits. |
| ///////////////////////////////////////////////////// | ||
|
|
||
| function orders (){ | ||
| let orders={}; |
|
|
||
|
|
||
| app.post('/api/order/', function(req, res){ | ||
| let orderId= `Order${id}` |
There was a problem hiding this comment.
It might simpler to just use the id rather than append Order string to it
| } | ||
|
|
||
| showMenu(event){ | ||
| document.querySelector(".app_menu").style.display="block" |
There was a problem hiding this comment.
Avoid using DOM manipulation in React. It would be better to set a value in state and then based on that add a class during render
| class Menu extends React.Component { | ||
| constructor(){ | ||
| super(); | ||
| this.state={menu:[], order:{}, total:0, orderId:1}; |
There was a problem hiding this comment.
Code would slightly more readable if each key/value pair was on own line
| fetch(`http://localhost:8080/menu`) | ||
| .then(function(response){ | ||
| return response.json(); | ||
| }).then(function(jsonData){ |
There was a problem hiding this comment.
If you make this an arrow function, you don't need self
| this.setState({total: myTotal}); | ||
| this.setState({order : allOrders}); | ||
|
|
||
| document.querySelector(".charge").style.display="block"; |
There was a problem hiding this comment.
Avoid DOM manipulation in React
| const self=this; | ||
| this.setState({orderId:1}); | ||
|
|
||
| fetch('http://localhost:8080/api/order', { |
There was a problem hiding this comment.
Use relative url here /api/order otherwise code will break when deployed to another domain name
| <p> Total: £{this.state.total} <label className="charge"> + £{this.state.total/20} delivery charge </label> </p> | ||
| <button onClick={this.submitOrder}> Submit Order! </button> | ||
| </div> | ||
|
|
There was a problem hiding this comment.
Indenation could be tidier
| orderAgain(event) | ||
| { | ||
| const self=this; | ||
| fetch('http://localhost:8080/api/order', { |
There was a problem hiding this comment.
Use relative url /api/order
| } | ||
| }).then(function(response) { | ||
| return response.json(); | ||
| }).then(function(data) { |
There was a problem hiding this comment.
use arrow function to avoid self
| }); | ||
|
|
||
|
|
||
| app.delete('/api/delete/', function(req, res){ |
There was a problem hiding this comment.
It would be better to use the route /api/order the delete islready implied by the HTTP method delete. The HTTP method is the action we are performing and the route name represents the resource we are performing the action on. Also, it would be good to use the id as a route param to indicate which item from orders is being deleted.
| fetchOrders(){ | ||
|
|
||
| const self=this; | ||
| fetch(`http://localhost:8080/orders`) |
| import Order from './Order'; | ||
|
|
||
| class Orders extends React.Component { | ||
| constructor(){ |
There was a problem hiding this comment.
could do with some indentation
No description provided.