-
Notifications
You must be signed in to change notification settings - Fork 60
Mikaelas js-project-recipe-library #66
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
| <button class="active">All</button> | ||
| <button>Starter</button> | ||
| <button>Main Course</button> | ||
| <button>Dessert</button> | ||
| <button>Side Dish</button> | ||
| <button>Lunch</button> | ||
| <button>Dinner</button> | ||
| </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 work! The button structure is clear and easy to follow!| if (dishTypes) { | ||
| filteredRecipes = filteredRecipes.filter(recipe => recipe.dishTypes.some(type => dishTypes.has(simplifyButtonText(type)))); | ||
| } if (cuisines) { | ||
| filteredRecipes = filteredRecipes.filter(recipe => recipe.cuisines.some(cuisine => cuisines.has(simplifyButtonText(cuisine)))); | ||
| } if (glutenFree) { | ||
| filteredRecipes = filteredRecipes.filter(recipe => recipe.glutenFree); | ||
| } if (currentSortOrder === 'fast') { | ||
| filteredRecipes = filteredRecipes.sort((a, b) => a.readyInMinutes - b.readyInMinutes); | ||
| } else if (currentSortOrder === 'slow') { | ||
| filteredRecipes = filteredRecipes.sort((a, b) => b.readyInMinutes - a.readyInMinutes); | ||
| } |
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 work here! 👏 I like how clearly the filtering logic is structured, and the chaining of conditions makes it easy to follow. The code is readable and straightforward, which is always a big plus. For the sorting section, maybe extract the comparator into a small helper function to avoid repeating readyInMinutes logic. Not critical, but it could help if you add more sort options later
Nice job keeping it concise! ✅
| sortButtons.forEach(button => { | ||
| button.addEventListener('click', () => { | ||
| sortButtons.forEach(button => button.classList.remove('active')); | ||
| button.classList.add('active'); | ||
|
|
||
| const sortOrder = button.textContent.trim().toLowerCase(); | ||
|
|
||
| currentSortOrder = sortOrder.includes('fastest') ? 'fast' : 'slow'; | ||
|
|
||
| applyFilters(); | ||
| }); | ||
| }); |
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 event handling is clear, and I like how the active class is managed—makes the UI state obvious and consistent. The trim().toLowerCase() approach is a nice touch to avoid issues with whitespace or capitalization )
| randomButton.addEventListener('click', () => { | ||
| randomButton.classList.toggle('active'); | ||
| randomButton.textContent = randomButton.classList.contains('active') ? 'Back to All' : 'Random recipe'; | ||
| const randomRecipe = currentRecipes[Math.floor(Math.random() * currentRecipes.length)]; | ||
|
|
||
| if (randomButton.classList.contains('active')) { | ||
| displayRecipes([randomRecipe]); | ||
| } else { | ||
| displayRecipes(currentRecipes); | ||
| } | ||
| }); No newline at end of file |
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 toggle logic is clear, and I like how the button text updates dynamically based on its state—it makes the UX intuitive. Using Math.floor(Math.random() * currentRecipes.length) is a straightforward way to pick a random recipe, and it’s easy to follow. Great job! 👏
| const localRecipes = [ | ||
| { | ||
| title: 'Focaccia Bread', | ||
| image: 'images/focaccia.jpg', | ||
| dishTypes: ['bread', 'side dish'], | ||
| cuisines: ['Italian', 'European'], | ||
| readyInMinutes: 45, | ||
| extendedIngredients: [ | ||
| { original: 'flour' }, | ||
| { original: 'olive oil' }, | ||
| { original: 'yeast' }, | ||
| { original: 'salt' } | ||
| ] |
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 recipe data is well-structured and easy to read. I like how each recipe object is consistent in shape, making it straightforward to work with for filtering, sorting, or displaying. The use of extendedIngredients as an array of objects is a smart choice for flexibility
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 job with this project!
Your code is easy to follow and well structured.
You have met the requirements.
I would like to see a bit more engagement in the commits. Even if you built this project fairly quickly and in more or less one go, work with commits for each section or feature that you work on. That way you can easier go back and find specific code snippets or logic tied to specific thing in a better way. Also, it's more valuable for the ones looking at your code (recruiters or peers) when they can follow your work flow a bit more
I added a few comments in the code. Take a look and keep up the good work!
| color: #0018A4; | ||
| font-size: 64px; | ||
| font-style: bold; | ||
| /* height: 85px; */ |
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.
remove unused code
| border-radius: 50px; | ||
| padding: 8px 16px; | ||
| margin: 0 0 10px 0; | ||
| border: none; |
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.
to get rid of that "jumping" of the buttons when you hover, try adding a transparent border to the buttons instead of a none border
border: solid 2px transparent
| display: flex; | ||
| flex-direction: column; | ||
| flex-wrap: wrap; |
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.
indentation
| <div class='card'> | ||
| <h3>Inga recept hittades</h3> | ||
| <p>Prova att filtrera på något annat.</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.
stick to english?
| <p><b>Dish type:</b> ${recipe.dishTypes.join(', ') || 'Unknown Dish Type'}</p> | ||
| <p><b>Cuisine:</b> ${recipe.cuisines.join(', ') || 'Unknown Cuisine'}</p> | ||
| <p><b>Time:</b> ${recipe.readyInMinutes || 'N/A'} minutes</p> |
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
| const handleError = error => { | ||
| alert("The API request limit has been reached. But don't worry, you can still view some local recipes!"); | ||
| currentRecipes = localRecipes; | ||
| displayRecipes(localRecipes); | ||
| } |
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.
⭐
https://mikaelas-js-project-recipe-library.netlify.app/