Skip to content
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

Bianka - recipe project #20

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open

Bianka - recipe project #20

wants to merge 39 commits into from

Conversation

Bianka2112
Copy link

Hi, thank you for your comments and feedback!
I am currently dealing with a strange issue for the random button where the live server is loading data even if its from localStorage or a local .js file but the deployed link does not seem to find the very same info. Am still troubleshooting that, so if nothing loads on the deployed site- please bear with me while I fix that asap. Otherwise most functionality is hopefully running! Will be continuing to update code through the weekend. Thanks!

B 🤓

@Lillebrorgroda
Copy link

Error message when I press Suprise

I get this when I press the random/surprise btn and all the recipes are still there so something is not working. I wanted you to know rigth away. I will continue with the rewiew and come back 😺

Copy link

@Lillebrorgroda Lillebrorgroda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’ve created a great-looking site that closely matches the design. I really loved the random button—the effect adds a nice touch to the experience. The error messages are not only funny but also make a lot of sense.

However, I noticed that one requirement is missing: the random button’s functionality. While the error messages work in the console, the button itself doesn’t seem to select and display a random recipe as intended.

Additionally, I think it would be helpful to add more comments explaining what different parts of the code are supposed to do. While the naming is clear, some sections are still a bit hard to follow, so a few extra explanations would make it even more readable.

Comment on lines +98 to +114
#random-recipe-btn.active {
background: linear-gradient(45deg, #ff0000, #ff7300, #ffeb00, #47ff00, #00ffb7, #0018A4, #7a00ff);
background-size: 200% 200%;
color: white;
text-shadow: 2px 2px 4px rgba(0, 0, 0, 0.5);
padding: 15px 20px;
font-weight: bold;
border-radius: 30px;
animation: gradientMove 5s ease-in-out infinite;
}

@keyframes gradientMove {
0% { background-position: 0% 50%; }
50% { background-position: 100% 50%; }
100% { background-position: 0% 50%; }
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is brilliant :) love the feature on this button.

script.js Outdated
Comment on lines 2 to 15
const messageBox = document.getElementById("message")

const pickAllFilter = document.getElementById("all")
const pickMexicanFilter = document.getElementById("mexican")
const pickMediterraneanFilter = document.getElementById("mediterranean")
const pickAsianFilter = document.getElementById("asian")
const filterButtons = document.querySelectorAll(".filtered, .sorted, #random-recipe-btn")
const randomRecipeBtn = document.getElementById("random-recipe-btn")

const container = document.getElementById("js-recipe-container")

// API RESOURCES
const randomURL = "https://api.spoonacular.com/recipes/random?apiKey=2c9fdce04f884694b4cef3682f7a3bba"
const recipesURL = "https://api.spoonacular.com/recipes/complexSearch?apiKey=2c9fdce04f884694b4cef3682f7a3bba&number=20&addRecipeInformation=true&cuisine=African,Asian,American,British,Cajun,Caribbean,Chinese,Eastern,European,European,French,German,Greek,Indian,Irish,Italian,Japanese,Jewish,Korean,Latin,American,Mediterranean,Mexican,Middle,Eastern,Nordic,Southern,Spanish,Thai,Vietnamese&fillIngredients=true&addRecipeInstructions=true"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Easy to understand and good structure.

script.js Outdated
Comment on lines 18 to 40
// Helper functions
const clearContainerHTML = () => {
container.innerHTML = ''
}

const updateMessage = (message) => {
messageBox.innerHTML = '' // Clear the message box
messageBox.innerHTML += `<p>${message}</p>`
}

const clearActiveButtons = () => {
filterButtons.forEach(button => {
button.classList.remove("active")
})
}

const activateButton = (button) => {
button.classList.toggle("active")
}

const displayNoResultsMessage = (message) => {
messageBox.innerHTML = `<p>${message}</p>`
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is aswell easy to understand. Step by step 😄

script.js Outdated
Comment on lines 42 to 99
// Fetch Default Recipes from API || Local Storage
const fetchAllRecipeData = async () => {
try {
const res = await fetch(recipesURL)
if (!res.ok) {
throw new Error(`HTTP error! Status: ${res.status}`)
}

const data = await res.json()
if (!data.results || data.results.length === 0) {
throw new Error("No results found in API response")
}

fetchedRecipesArray = data.results

loadRecipes(fetchedRecipesArray)
filterChoice(fetchedRecipesArray)

} catch (error) {
console.warn("API request failed, loading from localStorage...", error)
}

const savedData = localStorage.getItem("recipes")
if (savedData) {
const data = JSON.parse(savedData)

fetchedRecipesArray = data.results
loadRecipes(fetchedRecipesArray)
filterChoice(fetchedRecipesArray)

} else {
console.error("No saved recipes found in localStorage! loading from local file...")
}

if (window.savedRecipesData?.results?.length) {
const savedData = window.savedRecipesData.results || []

fetchedRecipesArray = savedData
loadRecipes(fetchedRecipesArray)
console.warn("All hope is not lost!🥳 savedFile saves the day")

}

if (typeof window.savedRecipesData === "string") {
try {
window.savedRecipesData = JSON.parse(window.savedRecipesData)
console.log("✅ Parsed `window.savedRecipesData` successfully.")

fetchedRecipesArray = savedData
loadRecipes(fetchedRecipesArray)
console.warn("All hope is not lost!🥳")

} catch (jsonError) {
console.error("❌ Failed to parse `window.savedRecipesData`:", jsonError)
}
console.error("No recipes found in anything! IT'S ALL BROKEN")
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error-message is superfun and they also explain the code in an informative way. I might understand the concept of the fetch due to this code snippet. Supercool that you also are doing a localStorage.

script.js Outdated
Comment on lines 179 to 209
const filterChoice = () => {

pickAllFilter.addEventListener("click", () => {
clearActiveButtons()
activateButton(pickAllFilter)
updateMessage("You eat everything, maybe liver then?")
loadRecipes(fetchedRecipesArray)
})

pickMexicanFilter.addEventListener("click", () => {
clearActiveButtons()
activateButton(pickMexicanFilter)
updateMessage("Yes. The answer is always tacos!")
filterByCuisine("mexican")
})

pickMediterraneanFilter.addEventListener("click", () => {
clearActiveButtons()
activateButton(pickMediterraneanFilter)
updateMessage("They say Mediterranean is the healthiest diet")
filterByCuisine("mediterranean")
})

pickAsianFilter.addEventListener("click", () => {
clearActiveButtons()
activateButton(pickAsianFilter)
updateMessage("你选择了中文")
filterByCuisine("asian")
})
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you have an opportunity to apply the DRY principle!

I didn’t know exactly how to do it, but I knew it was possible.

I achieved it using querySelectorAll and forEach. Additionally, I used dataset in my JavaScript file and data- attributes in my HTML file to avoid repeating the same code.

script.js Outdated
Comment on lines 215 to 228
ascendingButton.addEventListener("click", () => {
clearActiveButtons()
activateButton(ascendingButton)
updateMessage("What's the rush?")
loadRecipes([...fetchedRecipesArray].sort((a, b) => a.readyInMinutes - b.readyInMinutes))
})

descendingButton.addEventListener("click", () => {
clearActiveButtons()
activateButton(descendingButton)
updateMessage("Slow and steady = made with love")
loadRecipes([...fetchedRecipesArray].sort((a, b) => b.readyInMinutes - a.readyInMinutes))
})
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also ways to combine these two.

script.js Outdated
Comment on lines 231 to 263
const fetchRandomData = async () => {
try {
const res = await fetch(randomURL)
if (!res.ok) {
throw new Error(`HTTP error! Status: ${res.status}`)
}

const data = await res.json()
if (!data.results || data.results.length === 0) {
throw new Error("No results found in API response")
}

displayRandomRecipe(data.recipes[0])
//randomURL provides only one recipe so this is unique

} catch (error) {
console.warn("API request failed, loading from localStorage...", error)
}
const savedData = localStorage.getItem("recipes")
if (savedData) {
const data = JSON.parse(savedData)

const randomIndex = Math.floor(Math.random() * data.results.length)
const randomRecipe = fetchedRecipesArray[randomIndex]

displayRandomRecipe(randomRecipe)
console.warn("All hope is not lost!🥳 localStorage loaded")

} else {
console.error("No recipes found in API or localStorage! IT'S ALL BROKEN")
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of understand it, but if you have the opportunity to explain this part a bit more, that would be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants