Recipe Library (Oskar Nordin) #22
Conversation
…y to checking boolean data in object.
VariaSlu
left a comment
VariaSlu
left a comment
There was a problem hiding this comment.
Your CSS is well-structured and shows a lot of thoughtfulness, especially with the use of CSS variables like --primary-color and --secondary-color. This approach makes it easier to maintain and scale your design as the project grows. The layout is clean, and the responsiveness through media queries works well. Elements like .errorMessages and .errorButton not only look good but also feel intuitive for the user.
One thing to think about: if no recipes match a filter or search, the app doesn’t give the user any feedback. Maybe adding a simple message like “No recipes found. Try adjusting your search or filters!” would help guide the user and make the experience feel smoother. Overall, you’re doing a great job, and it’s clear you’re putting care into the details. Keep building on that!
index.html
Outdated
|
|
||
|
|
||
| <script src="script.js"></script> | ||
| </form> |
There was a problem hiding this comment.
There’s an unnecessary closing tag at the very end of the file that doesn’t correspond to any opening
tag.
index.html
Outdated
| </label> | ||
| </div> | ||
| </div> | ||
| <div class="container"> |
There was a problem hiding this comment.
If it's a placeholder for dynamically loaded recipes, consider adding a comment to clarify its purpose :)
index.html
Outdated
| <link rel="stylesheet" href="style.css"> | ||
| <link rel="preconnect" href="https://fonts.googleapis.com"> | ||
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin> | ||
| <link href="https://fonts.googleapis.com/css2?family=Kumbh+Sans:[email protected]&display=swap" rel="stylesheet"> |
There was a problem hiding this comment.
I like your choice of the custom font (Kumbh Sans), it is adding to the visual appeal for sure!
| loadRecipes(filteredRecipes) | ||
| } | ||
|
|
||
| // Show loader. Hide after 2 seconds. |
There was a problem hiding this comment.
I've noticed the loader is hidden after 2 seconds using setTimeout, which may not align with the actual data loading time. This can cause inconsistent UX. It seems possible to dynamically hide the loader once the fetch completes
showLoader(); fetch(URL) .then(() => hideLoader()) .catch(() => hideLoader());
|
Can you share a link to your deployed page as well please 😇 |
|
… On Wed, 26 Mar 2025 at 09:39, Matilda Brunemalm ***@***.***> wrote:
Can you share a link to your deployed page as well please 😇
—
Reply to this email directly, view it on GitHub
<#22 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJQPVURZWFBTEV7FYGAYFLD2WJKUJAVCNFSM6AAAAABZBSB2IOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJTGYYDMNBQGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
[image: HIPPIEKICK]*HIPPIEKICK* left a comment
(Technigo/js-project-recipe-library#22)
<#22 (comment)>
Can you share a link to your deployed page as well please 😇
—
Reply to this email directly, view it on GitHub
<#22 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJQPVURZWFBTEV7FYGAYFLD2WJKUJAVCNFSM6AAAAABZBSB2IOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJTGYYDMNBQGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
HIPPIEKICK
left a comment
HIPPIEKICK
left a comment
There was a problem hiding this comment.
Good job with this project Oskar! Although I have some things for you to work on, I think you should be proud of what you’ve accomplished. First, I’d like to ask you to have another look at how your page looks on mobile. It doesn’t really fit. An idea would be to decrease the padding a bit to let the content take a little bit more width (we don’t have so much width to work on on mobile to start with 😅)
Then I would also like you to clean up your code:
- Be consistent with whether or not you’re using semicolons.
- Remove console.log from production code.
- Set your VSC tab to 2 spaces. It’s so hard to read with 4 spaces 🙈
And I noticed that you completed some of the stretch goals (great job!) but your loader is not showing as expected, so please have a look at that as well if you’re aiming for VG.
|
Thanks for the feedback. I'm working on fixing the suggestions you
mentioned. Didn't even notice the 4-space indentation. 💀💀💀 Will also
check the loading stretch goal.
…On Thu, 27 Mar 2025 at 15:22, Matilda Brunemalm ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Good job with this project Oskar! Although I have some things for you to
work on, I think you should be proud of what you’ve accomplished. First,
I’d like to ask you to have another look at how your page looks on mobile.
It doesn’t really fit. An idea would be to decrease the padding a bit to
let the content take a little bit more width (we don’t have so much width
to work on on mobile to start with 😅)
Then I would also like you to clean up your code:
- Be consistent with whether or not you’re using semicolons.
- Remove console.log from production code.
- Set your VSC tab to 2 spaces. It’s so hard to read with 4 spaces 🙈
And I noticed that you completed some of the stretch goals (great job!)
but your loader is not showing as expected, so please have a look at that
as well if you’re aiming for VG.
—
Reply to this email directly, view it on GitHub
<#22 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJQPVUWJHW6LS2WILFM3MOT2WP3QVAVCNFSM6AAAAABZBSB2IOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMRRHA4DGNRUGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
…e loading feature work.
|
- Be consistent with whether or not you’re using semicolons.
Removed them.
- Remove console.log from production code. Removed them.
- Set your VSC tab to 2 spaces. It’s so hard to read with 4 spaces Set
it to 2 spaces.
- + Made the loading feature work.
https://recipe-libary-on.netlify.app/
// Oskar
…On Thu, 27 Mar 2025 at 17:03, Oskar Nordin ***@***.***> wrote:
Thanks for the feedback. I'm working on fixing the suggestions you
mentioned. Didn't even notice the 4-space indentation. 💀💀💀 Will also
check the loading stretch goal.
On Thu, 27 Mar 2025 at 15:22, Matilda Brunemalm ***@***.***>
wrote:
> ***@***.**** requested changes on this pull request.
>
> Good job with this project Oskar! Although I have some things for you to
> work on, I think you should be proud of what you’ve accomplished. First,
> I’d like to ask you to have another look at how your page looks on mobile.
> It doesn’t really fit. An idea would be to decrease the padding a bit to
> let the content take a little bit more width (we don’t have so much width
> to work on on mobile to start with 😅)
>
> Then I would also like you to clean up your code:
>
> - Be consistent with whether or not you’re using semicolons.
> - Remove console.log from production code.
> - Set your VSC tab to 2 spaces. It’s so hard to read with 4 spaces 🙈
>
> And I noticed that you completed some of the stretch goals (great job!)
> but your loader is not showing as expected, so please have a look at that
> as well if you’re aiming for VG.
>
> —
> Reply to this email directly, view it on GitHub
> <#22 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AJQPVUWJHW6LS2WILFM3MOT2WP3QVAVCNFSM6AAAAABZBSB2IOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMRRHA4DGNRUGY>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
3 participants
No description provided.