Tavan's Recipe Library #28
Conversation
…tent to look nice on different screens
…se it will only show the numbbers
… when user clicks on the filtering buttons
…epending on cooking time
…me to title and adding servings
…, sorting and random button work properly
There was a problem hiding this comment.
I think you did a great job! I like the colors and styling you used. I am super impressed with how clean your code looks and simple it is to read. I think there's only 2 details to update to meet the requirements: a message when no recipes match a filter and if your API limit is reached. But otherwise I think you got everything required.
Another detail if you have time is getting the buttons to clear when others are selected and adding some type of backup data if your API limit is reached but I do not think (I think) these are requirements, just good to haves.
In all cases, congrats on a nice site!
script.js
Outdated
| // Filter valid recipes | ||
| allRecipes = data.recipes.filter((recipe) => { | ||
| return recipe.cuisines.length > 0 && recipe.image && recipe.title; | ||
| }); |
There was a problem hiding this comment.
I like how you filtered your data to get matches from the beginning! This looks super neat and a great implementation.
There was a problem hiding this comment.
Thank you! I decided to filter it from the beginning. Wasn't sure if this was the right approach but went for it.
| const oldContainer = document.getElementById("recipes-container"); | ||
| if (oldContainer) { | ||
| oldContainer.remove(); | ||
| } | ||
|
|
||
| const container = document.createElement("div"); | ||
| container.id = "recipes-container"; |
There was a problem hiding this comment.
I know you mentioned this- but just wondering if its because you have an html written 'recipes-container'? Maybe you dont need it anymore if you have all that old HTML code commented out?
| <ul> | ||
| ${Array.isArray(recipe.extendedIngredients) | ||
| ? recipe.extendedIngredients.map((item) => `<li>${item.original}</li>`).join("") | ||
| : "<li>No ingredients available</li>" | ||
| } | ||
| </ul> |
There was a problem hiding this comment.
Again, love how you wrote this code. It is SO NEAT and minimal 😍 I might have to refactor mine lol. This is very pleasing to find a super easy way to code this out and it is super simple to understand. 🙌
There was a problem hiding this comment.
Thank you!🙌 I'm glad to hear that since I think I always try to keep it minimal and simple to understand.
script.js
Outdated
| // Random recipe button | ||
| document.querySelector('.random-button').addEventListener('click', (event) => { | ||
| document.querySelectorAll('button').forEach(btn => btn.classList.remove('selected')); |
There was a problem hiding this comment.
I know we discussed the random button not reset/clear other selected buttons. Two notes here:
- I see that maybe you've mixed using "", '' double quotes in codes above and single quotes here. Not sure that has any effect, but could help in maintaining consistency on one or the other.
- Could it be that 'button' is not selecting all your buttons? I've heard that adding
type=buttonis good practice especially for buttons that do not submit anything to the page. Like we are doing here. Another idea is in my code I wrote a global helper function to clear buttons that I could call later throughout my code and maybe that's another approach to consider here. But I find your code extremely efficient already, so I think you know better than I!!
There was a problem hiding this comment.
It is good that you're pointing this out since I myself haven't even noticed the inconsistent use of " " and ' '. Also about the global helper function, I haven't thought of that. Tried to solve it in other ways but not this one. Will also take another look at the type=button. Thank you so much for the help!
style.css
Outdated
| } | ||
| } | ||
|
|
||
| /* Media Querie Mobile Device */ |
There was a problem hiding this comment.
I can recommend renaming the comment to quickly see which media query goes to which type of device, since these dimensions are more likely a computer/laptop rather than mobile device. But great job, again, on super clean and concise coding!
There was a problem hiding this comment.
Thank you! Good point, forgot to change the names!
There was a problem hiding this comment.
I happened to watch your site on my big screen (that is bigger than 1600px) and I noticed you don’t have a grid on bigger screens. Although not a requirement, you might want to look into just extending the media query so that your grid is kept on bigger screens as well.
Apart from that I think your site looks good and the code as well ⭐ Keep up the good work!
Website contains following for now:
Error message will be added later
Link to website: https://tt-recipe-library.netlify.app