-
Notifications
You must be signed in to change notification settings - Fork 60
My Recipe Library---https://saras-js-project-recipe-library.netlify.app/ #61
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
base: main
Are you sure you want to change the base?
Conversation
julialindstrand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your website looks great overall, just added a suggestion for the styling but it looks like it has everything it needed and is presented in a really nice way. I really like your take on the messages, it looks fun and professional! Good job Sara! :D
I don't personally fell that comfortable with the script part yet so I don't feel entitled to have opinions and I'm not confident enough to say I would find a problem if there was one
| <main> | ||
| <h1>Recipe Library</h1> | ||
|
|
||
| <!-- ====== FILTERS / SORT / LUCKY CONTROLS ====== --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really neat way to organize your code!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you:)
| @@ -0,0 +1,442 @@ | |||
| /* ========== GLOBAL STYLES ========== */ | |||
| * { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really smart to set everything at zero to have a blank slate!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is a safety net of sort.
| :root { | ||
| --page-bg: #fafafa; | ||
| --blue-main: #0018a4; | ||
| --blue-outline: #0044cc; | ||
| --mint-green: #c0f8d1; | ||
| --pink-bg: #fddddf; | ||
| --text-blue: #0033aa; | ||
| } | ||
|
|
||
| body { | ||
| background: var(--page-bg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So when you use the root you "select" it with var?
style.css
Outdated
| font-family: "Futura", "Trebuchet MS", Arial, sans-serif; | ||
| } | ||
|
|
||
| .main-content { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is main-content? I can't seem to find it in the HTML. Or is that a gather-word for everything in the main?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that was a leftover from previous design, i have removed it now:) good eye
| /* ========== HEADINGS ========== */ | ||
| h1 { | ||
| font-family: "Futura", "Trebuchet MS", Arial, sans-serif; | ||
| font-size: 64px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the inspector it looks like the font-size dosen't matter, what defines the size?
| const params = new URLSearchParams({ | ||
| number: 30, | ||
| cuisine: 'Asian,Italian,Mexican,Middle Eastern', | ||
| diet: 'vegan,vegetarian,gluten free', | ||
| type: 'breakfast,main course,dessert', | ||
| sort: 'popularity', | ||
| addRecipeInformation: true, | ||
| instructionsRequired: true, | ||
| fillIngredients: true, | ||
| apiKey: API_KEY2 | ||
| }) | ||
|
|
||
| const URL = `https://api.spoonacular.com/recipes/complexSearch?${params.toString()}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so smart! And really easy to understand 💡
| <div class='card-content'> | ||
| <h3>${recipe.title}</h3> | ||
| <div class='divider'></div> | ||
| <p><strong>Cuisine:</strong> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of is intuitive since it might be read out loud of accessibility reasons
| <p class='ingredients-header'>Ingredients:</p> | ||
| <ul> | ||
| ${recipe?.extendedIngredients | ||
| .map(({ name }) => `<li>${name}</li>`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looked nice to have the list with dots
|
|
||
| if (cachedData) { | ||
| try { | ||
| const { recipes, timestamp } = JSON.parse(cachedData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the timestamp do?
| .map((cuisine) => cuisine.toLowerCase()) | ||
| .includes(selectedCuisine.toLowerCase()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smart to use toLowerCase if anything has a capital letter somewhere, to make sure nothings missed
JennieDalgren
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job with this project!
You code is clean and structured and easy to follow. The site looks really well worked on and the things you have implemented you have done so very good. Thinking about the requirements, the styling and the small extras such as the animation.
Keep up the good work!
| const params = new URLSearchParams({ | ||
| number: 30, | ||
| cuisine: 'Asian,Italian,Mexican,Middle Eastern', | ||
| diet: 'vegan,vegetarian,gluten free', | ||
| type: 'breakfast,main course,dessert', | ||
| sort: 'popularity', | ||
| addRecipeInformation: true, | ||
| instructionsRequired: true, | ||
| fillIngredients: true, | ||
| apiKey: API_KEY | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice that you have used URLSearchParams! Super useful and clever
| <p>We couldn't find any recipes for your filter...<br> | ||
| Try changing it or click <strong>Surprise me!</strong> 🎉</p> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice empty state msg and nudge to try the random button :)
| ${recipe?.cuisines?.length ? recipe.cuisines.join(', ') : 'N/A'} | ||
| </span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good fallback
| if (cachedData) { | ||
| try { | ||
| const { recipes, timestamp } = JSON.parse(cachedData) | ||
| const isExpired = Date.now() - timestamp > CACHE_DURATION | ||
| recipesArray = recipes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
smart, nice use of a cache "timer" too
| if (res.status === 402) { | ||
| cardsContainer.innerHTML = ` | ||
| <div class='empty-message'> | ||
| <h2>Daily API limit reached 🚫</h2> | ||
| <p>Please try again later.</p> | ||
| </div>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⭐
| const sortOnTime = () => { | ||
| const selected = sortSection.querySelector('input[name="order"]:checked') | ||
| const order = selected ? selected.value : 'desc' | ||
|
|
||
| const sorted = [...recipesArray].sort((a, b) => { | ||
| return order === 'asc' | ||
| ? a.readyInMinutes - b.readyInMinutes | ||
| : b.readyInMinutes - a.readyInMinutes | ||
| }) | ||
|
|
||
| showCardsContainer(sorted) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice job! If you want to keep the sorting even when you change the cuisine filters you could maybe save the sorting rule in a global variable. Just something to take with you going forward.
| --page-bg: #fafafa; | ||
| --blue-main: #0018a4; | ||
| --blue-outline: #0044cc; | ||
| --mint-green: #c0f8d1; | ||
| --pink-bg: #fddddf; | ||
| --text-blue: #0033aa; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yay! css variables 🎨
| animation: pop-in 0.5s ease; | ||
| } | ||
|
|
||
| /* ========== ANIMATIONS ========== */ | ||
| @keyframes pop-in { | ||
| 0% { | ||
| transform: scale(0.8); | ||
| opacity: 0; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wohoo! Nice animation. It's the small things that really makes it pop :)
My Netlify link: saras-js-project-recipe-library.netlify.app