-
Notifications
You must be signed in to change notification settings - Fork 60
Jennifer-Recipe library project #46
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
|
Honestly It looks great. I can't find anything to comment on. |
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!
It feels like you have maybe over engineered this project a bit, and that is fine for learning purpose. I'm not sure how much you understand about the code and if you were to think of all the steps for local storage, stale data etc on your own without the help of copilot or similar. It is totally fine if you do. Just remember that you need to understand the code you have written.
Good job with the project all in all, I really like the tiny little detail of showing how many recipes there is in each filtered view.
Keep up the good work! 💪
| :root { | ||
| --bg: #f6f9ff; | ||
| --surface: #ffffff; | ||
| --ink: #0f172a; | ||
| --muted: #475569; | ||
| --stroke: #E9E9E9; | ||
| --brand-heading: #1e3a8a; | ||
| --blue: #0018A4; | ||
| --blue-contrast: #ffffff; | ||
| --pink: #FFECEA; | ||
| --pink-strong: #FF5C8A; | ||
| --mint: #CCFFE2; | ||
| --overlay: rgba(15, 23, 42, .45); | ||
| --shadow-card: 0 10px 20px rgba(2, 6, 23, .07); |
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
| /* =========================================================== | ||
| Recipe Finder App | ||
| Code is split up into small, named functions that describe what they do(purpose). | ||
| Each section is clearly marked with a header. | ||
|
|
||
| Sections: | ||
| -1 API config & fetch | ||
| -2 Cache handling | ||
| -3 DOM helpers | ||
| -4 String helpers | ||
| -5 Normalization helpers | ||
| -6 Cache functions | ||
| -7 Fetch with quota handling | ||
| -8 Filtering & sorting | ||
| -9 Rendering | ||
| -10 Event handling & init + random recipe | ||
| =========================================================== */ | ||
|
|
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 documentation!
| // $() → shortcut for document.getElementById() | ||
| // grid → reference to <section id="grid"> where recipe cards go |
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!
| // grid → reference to <section id="grid"> where recipe cards go | ||
| const $ = (id) => document.getElementById(id); | ||
| const grid = $('grid'); | ||
| const setBusy = (on) => grid.toggleAttribute('aria-busy', !!on); |
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 this part used for?
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 my code it's used to improve accessibility for screen reader users. it basically tells assistive technologies (reads out the text and structure of the webpage for people who are blind) that the recipe grid is currently loadin or updating content.
I have been told that this is important to add to webpages to make it accessable for everyone, but thank you for your feedback.
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! It is super important to think about accessibility aspect. You will learn even more about that this week and the next
Netlify link: library-recipe.netlify.app
Hello Pebbles!
Looking forward for your code review! More changes will come, still working on spoonacular API url and have not cleaned code yet.
Have a great day!