Javascript2 week2/soheib #2
Conversation
|
Amazing work! |
|
|
||
|
|
||
| //a function to render a preview card for each recipe inside a given container "parent" | ||
| const renderRecipeCard = (parent, array) =>{ |
There was a problem hiding this comment.
nitpicking; array is a very generic term. I would call array => receipes
There was a problem hiding this comment.
Thank you for pointing this out. It makes much more sense
| //a function to render a preview card for each recipe inside a given container "parent" | ||
| const renderRecipeCard = (parent, array) =>{ | ||
| parent.innerHTML = "" | ||
| array.forEach((obj)=>{ |
There was a problem hiding this comment.
same here
receipes.forEach( receipe => {})
| obj.ingredients.forEach((ingredient) => { | ||
| const listItem = document.createElement('li'); | ||
| listItem.innerText = ingredient.amount > 0 | ||
| ? `${ingredient.amount || ""} ${ingredient.unit || ""} of ${ingredient.name}` |
There was a problem hiding this comment.
It's a good use of ternary operator. I would suggest however to switch this into an if statement to avoid having to compile a long line in your head to understand the conditions of this ternary operation
| parent.appendChild(recipeCard) | ||
|
|
||
| //making cards expand on click | ||
| recipeCard.addEventListener("click", ()=>{ |
There was a problem hiding this comment.
should we move the content of this click event listener into a separate function?
|
|
||
|
|
||
| ingredientSearchBtn.addEventListener("click", (e)=> { | ||
| const num = Math.sqrt((parseInt(ingredientNumber.value) ** 2)); // because I'm lazy I am getting the square root of the square of the input, to make sure it's not a negative value |
There was a problem hiding this comment.
I suggest to do it properly by checking the negative number :)
| }; | ||
| // check for the value if it's an array to handle the case of ingredients | ||
| if ( Array.isArray(value)) { | ||
| value.forEach((element)=>{ |
There was a problem hiding this comment.
what is an element here? should give it a more concrete name
There was a problem hiding this comment.
Same mistake as "array" earlier, I was using names to remind me types, but I will stick to descriptive one from now on
|
|
||
|
|
||
|
|
||
| // finding recipes by keywords |
There was a problem hiding this comment.
Add a more detailed comment here explaining where do you search about the keyword
|
|
||
| seconds++; | ||
| timeCount.innerText = seconds; | ||
| durationUnit.textContent = " seconds"; |
There was a problem hiding this comment.
what are you trying to do here?
Another approach you can follow is to have new Date() object to track the date when you started visiting the website and then you compare it to the current date to get the duration (the time difference)
There was a problem hiding this comment.
That is a lot smarter than what I was doing. Thanks a million
| } | ||
|
|
||
| setTimerBtn.addEventListener("click", ()=>{ | ||
| const timeAmount = Math.sqrt((parseInt(minutesInput.value) ** 2)) |
There was a problem hiding this comment.
Same as earlier, just because I didn't want to do a regex for negative numbers. but I will do it proper 🥸
|
|
||
| setTimerBtn.addEventListener("click", ()=>{ | ||
| const timeAmount = Math.sqrt((parseInt(minutesInput.value) ** 2)) | ||
| const timeInMlSec = timeAmount * 60000 // converting the amount of time to milliseconds |
Dear mentor,
Please disregard "scripts.js" and only look at "script.js" and note that this pull request includes week2 + week3 assignments.
I have created a new file, since the changes were too important, and I preferred going on a cleaner basis. The only reason the old file is still there is because I still have some old features I'm still migrating.
I am still laking a UI with the css, but I'll focus on that soon.
Thank you again for your precious time and attention.
This is not just homework for me, but it is more and more morphing into a project I'm getting passionate about. (I know I'm still new and naive haha)