pizza builder - Dani Demarchi #726
Conversation
| // $(".btn-mushrooms").removeClass("active"); | ||
|
|
||
| // $(".btn-pepperonni").removeClass("active"); | ||
|
|
There was a problem hiding this comment.
Please make sure you do a "clean up" commit before you submit a pull request. Get rid of all unnecessary comments, extra empty lines (you should typically be using 1 at most) and any other unused variables or functions
lienekechee
left a comment
lienekechee
left a comment
There was a problem hiding this comment.
Nice work Dani! Make sure to submit clean versions of your work though :)
|
|
||
| //setting default price on load | ||
| $(".total").html("13"); | ||
|
|
There was a problem hiding this comment.
I think it would be better (more explicit) to have a function called setupDefaultView() or something that shows what you are doing.
| } else { | ||
| $(".total").html(parseFloat(total) - 5); | ||
| } | ||
|
|
There was a problem hiding this comment.
Perhaps it might be an idea to declare the variables total and className globally, reassign them on click then write a function that takes in these two variables and updates $(".total"). This way you can avoid duplicate code 😉
| <li id="li-3">$1 green peppers</li> | ||
| <li id="li-4">$3 white sauce</li> | ||
| <li id="li-5">$5 gluten-free crust</li> | ||
| </ul> |
There was a problem hiding this comment.
I think naming these li-1 is a wee bit redundant, you are able to select them using the right query methods (e.g. eq(i) which gets the element at index i. If anything, I would give them a class that corresponds to the ingredient or to the ingredient button, if you find that useful in your implementation.
2 participants
Here we go, guys.
Thanks for the help Matt! :)
Kind regards,
Dani