Recipe library #63
Recipe library #63
Conversation
…rd, html, and css
Demijuls
left a comment
Demijuls
left a comment
There was a problem hiding this comment.
Great job, very impressed with how simple and clean the code is!
|
|
||
| /* Layout */ | ||
| --container-max: 1200px; | ||
| --page-pad: 16px; |
There was a problem hiding this comment.
Great use of semantic tokens! Easy to read
| </h1> | ||
|
|
||
| <!-- Buttons för filter av --> | ||
| <section class="buttonarea"> |
There was a problem hiding this comment.
Great naming of sections, very easy to read!
| <section class="buttonarea"> | ||
| <div class="filter" aria-label="Filter & Sort"> | ||
| <div class="group"> | ||
| <div class="btn-row"> |
|
|
||
|
|
||
| <!-- Recipe Cards --> | ||
| <section class="recipe-cards" id="recipe-cards"> |
| --page-pad: 16px; | ||
| } | ||
|
|
||
| * { |
There was a problem hiding this comment.
I also like to use * as a way to make it easier to style everything after, but since it might slow down the loading of the site, I would probably just double-check that this property is really a must one
| .btn:hover, .btn-time:hover, .btn-random:hover{ | ||
| border-color: var(--h1); | ||
| } | ||
| .btn:active, .btn-time:active, .btn-random:active{ |
There was a problem hiding this comment.
Adding active state overall is a great idea for better UX, however, here the change feels a little bit too subtle maybe? Although it is not critical, just a small thing
| filterBtns.forEach(btn => { | ||
| btn.addEventListener('click', () => { | ||
| currentCuisine = btn.dataset.cuisine || 'all'; | ||
| setActive(filterBtns, btn); |
There was a problem hiding this comment.
Maybe also consider adding .active state to CSS to indicate to users which filter is currently in use after they pressed it? It's a small UX improvement though, the code looks very nice and clean to me!
| sortBtns: buttons that set up/down by time | ||
| randomBtn: the button that shows a random recipe | ||
| */ | ||
| const grid = document.querySelector('.recipe-grid'); |
There was a problem hiding this comment.
Great work with namings for filterBtn, sortBtn, randomBtn, very self-explanatory! Maybe grid could follow the same pattern? It might be a "fall-back" for human reading in the future:)
| console.log('[SORT CLICK]', { dataset: btn.dataset.sort, currentSortDir }); | ||
| const testUrl = buildUrl({ cuisine: currentCuisine, sortDir: currentSortDir, number: 14 }); | ||
| console.log('[URL]', testUrl); | ||
| setActive(sortBtns, btn); |
There was a problem hiding this comment.
Overall great JS work, very clean, simple, short and so easy to follow!
JennieDalgren
left a comment
JennieDalgren
left a comment
There was a problem hiding this comment.
Great job with this project!
Your code is well structured snd easy to follow. Remember to stick to english in your comments through out the project. It makes it easier for the reviewers to read your code.
You have met the requirements and some stretch goals too.
Keep up the good work!
| --bg: #FAFBFF; | ||
| --h1: #0018A4; | ||
| --h2: black; | ||
| --h3: #6F6F6F; | ||
| --button1: rgb(204, 255, 226); | ||
| --button2: rgb(255, 236, 234); |
| color: var(--h1); | ||
| font-size: 65px; | ||
| font-weight: bold; | ||
| /* margin: 50px 0; */ |
| .buttonarea, .filter{ | ||
| display: block; /* block på mobil */ | ||
| width: 100%; /* släpp var(--group-width) */ | ||
| /* height: auto; <-- implicit */ | ||
| box-sizing: border-box; |
There was a problem hiding this comment.
remove the comments, or translate them to english if you need them
| padding-left: max(clamp(12px, 6vw, 20px), env(safe-area-inset-left)); | ||
| padding-right: max(clamp(12px, 6vw, 20px), env(safe-area-inset-right)); |
There was a problem hiding this comment.
are you comfortable explaining what this styling does? if not, this might be a good place to add some comments that explains it.
| transition: transform .15s ease, box-shadow .15s ease, border-color .15s ease; | ||
| } | ||
|
|
||
| .recipe-card:hover { | ||
| border-color: var(--h1); | ||
| transform: translateY(-2px); | ||
| box-shadow: 0 4px 12px var(--recipe-shadow-hover); | ||
| } |
There was a problem hiding this comment.
smooth subtle animation on the hover
| const params = new URLSearchParams({ | ||
| apiKey: API_KEY, | ||
| number: String(number), | ||
| addRecipeInformation: "true", | ||
| instructionsRequired: "true", | ||
| sort: "time", | ||
| sortDirection: sortDir, | ||
| }); |
| /* | ||
| STATE | ||
| Variables that tell us what the user has selected | ||
| (kitchen and sorting) and which recipes were most recently retrieved. | ||
| When you click the buttons, these values are updated – and we retrieve new recipes. | ||
| */ | ||
| let currentCuisine = 'all'; | ||
| let currentSortDir = 'asc'; | ||
| let currentRecipes = []; |
There was a problem hiding this comment.
good use of comments and global variables
| document.querySelector('.btn[data-cuisine="all"]')?.classList.add('active'); | ||
| document.querySelector('.btn-time[data-sort="asc"]')?.classList.add('active'); |
There was a problem hiding this comment.
if these are active as default, you could set them in the html straight away
3 participants
No description provided.