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

getting rid of global state #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 30 additions & 38 deletions code/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const cityInput = document.getElementById("cityInput");


// Global variables
let city = "";
const timezoneOffset = new Date().getTimezoneOffset() * 60;

// The function creates an image for Javascript injection later into the HTML
Expand Down Expand Up @@ -42,46 +41,47 @@ const sunsetIcon = createWeatherImg("./assets/sunset-icon.png", "sunset");


function createFiveDayForecast(filteredForecast) {
weatherContainer.innerHTML = ""
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Clearing the HTML for the five days as the first thing before rewriting it (it was in the event listener before)

for (let i = 0; i < 5; i++) {

chooseWeatherIcon(filteredForecast, i);
const daysWeatherIcon = chooseWeatherIcon(filteredForecast[i]);
const days = new Date(filteredForecast[i].dt_txt).toLocaleDateString(
"en-US",
{ weekday: "long" }
);
const forecastTemp = Math.round(filteredForecast[i].main.temp * 10) / 10;
const forecastWeather = filteredForecast[i].weather.description;
createFiveDaysInnerHTML(days, forecastTemp);
}
weatherContainer.innerHTML += createFiveDaysInnerHTML(days, forecastTemp, daysWeatherIcon);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the function now returns the HTML (instead of assigning it to weatherContainer.InnerHTML) the assignment now happens here.

};
};

//Creates a weather icon based on the forecasted weather
function chooseWeatherIcon(filteredForecast, i) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this function didn't actually need two arguments, so we've just given it each day's forecast (from 'filteredForcast[i]) instead

let main = filteredForecast[i].weather[0].main;
function chooseWeatherIcon(forecast) {
let main = forecast.weather[0].main;
if (main === "Clouds") {
weatherImg = cloudyIcon;
return cloudyIcon;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the function now returns an icon, which we assign to a variable up in the place we called it. (instead of making the assignment inside the function)

} else if (main === "Rain") {
weatherImg = rainIcon;
return rainIcon;
} else if (main === "Thunderstorm") {
weatherImg = stormIcon;
return stormIcon;
} else if (main === "Drizzle") {
weatherImg = rainIcon;
return rainIcon;
} else if (main === "Fog" || main === "Mist") {
weatherImg = fogIcon;
return fogIcon;
} else if (main === "Snow") {
weatherImg = snowIcon;
return snowIcon;
} else {
weatherImg = sunIcon;
return sunIcon;
}
};

//Creates innerHTML for each day in the forecast
function createFiveDaysInnerHTML(days, forecastTemp) {
weatherContainer.innerHTML += `
function createFiveDaysInnerHTML(days, forecastTemp, daysWeatherIcon) {
return `
<section class="day">
<span class="forcast-day">${days}</span>
<span class="forcast-temp">${forecastTemp} °C </span>
<span class="forcast-weather"><img src=${weatherImg.src} alt=${weatherImg.alt} class="forecast-weather"/></span>
<span class="forcast-weather"><img src=${daysWeatherIcon.src} alt=${daysWeatherIcon.alt} class="forecast-weather"/></span>
</section>

`;
Expand All @@ -95,12 +95,12 @@ function getTimeOf(time, timezone) {
};

//Creates innerHTML to display todays weather and temperatures in the header
function changeHeaderInnerHTML(data, timeOfSunrise, timeOfSunset) {
today.innerHTML = `
function getHeaderHTML(data, timeOfSunrise, timeOfSunset, weatherIcon) {
return `
<h1>${data.name}</h1>
<h2>${Math.round(data.main.temp * 10) / 10} °C</h2>
<h5>Feels like: ${Math.round(data.main.feels_like * 10) / 10} °C</h5>
<img src=${weatherImg.src} alt=${weatherImg.alt} class= "today-weather"/>
<img src=${weatherIcon.src} alt=${weatherIcon.alt} class= "today-weather"/>
<h3>${data.weather.map((item) => item.description)}</h3>
<h5 class="windpar"><img src=${windIcon.src} alt=${windIcon.alt} class= "wind-icon"/> ${Math.round(data.wind.speed * 10) / 10} m/s</h5>
<p class="sunrisepar"><img src=${sunriseIcon.src} alt=${sunriseIcon.alt} class= "sunrise"/>Sunrise: ${timeOfSunrise}</p>
Expand All @@ -109,36 +109,27 @@ function changeHeaderInnerHTML(data, timeOfSunrise, timeOfSunset) {
};

//Changes background image in header depending on todays weather prognosis
function setBackgroundWeather(data) {
function getBackgroundWeatherClass(data) {
let main = data.weather.map((item) => item.main);
if (main.includes("Clouds")) {
weatherImg = cloudyIcon;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All of these are taken out because the other if/else function was doing the same thing

backgroundImg.classList = "clouds";
return "clouds";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these now return instead of assigning, the assigning is done when we call the function

} else if (main.includes("Rain")) {
weatherImg = rainIcon;
backgroundImg.classList = "rain";
return "rain";
} else if (main.includes("Drizzle")) {
weatherImg = rainIcon;
backgroundImg.classList = "drizzle";
return "drizzle";
} else if (main.includes("Snow")) {
weatherImg = snowIcon;
backgroundImg.classList = "snow";
return "snow";
} else if (main.includes("Fog") || (main.includes("Mist"))) {
weatherImg = fogIcon;
backgroundImg.classList = "fog", "mist";
return "fog";
} else if (main.includes("Clear")) {
weatherImg = sunIcon;
backgroundImg.classList = "clear";
return "clear";
} else {
weatherImg = atmosphereIcon;
backgroundImg.classList = "atmosphere";
return "atmosphere";
}
};
//Eventlisteners
// DON"T DELETE, TO USE LATER!
cityBtn.addEventListener('click', (e) => {
today.innerHTML = ""
weatherContainer.innerHTML = ""
let city = cityInput.value
console.log(city)
let WEATHER_API = `https://api.openweathermap.org/data/2.5/weather?q=${city}&units=metric&APPID=5000cd66a9090b2b62f53ce8a59ebd9e`;
Expand All @@ -148,11 +139,12 @@ cityBtn.addEventListener('click', (e) => {
fetch(WEATHER_API)
.then((res) => res.json())
.then((data) => {
setBackgroundWeather(data);
backgroundImg.classList = getBackgroundWeatherClass(data);
let timezone = data.timezone
const timeOfSunrise = getTimeOf(data.sys.sunrise, timezone);
const timeOfSunset = getTimeOf(data.sys.sunset, timezone)
changeHeaderInnerHTML(data, timeOfSunrise, timeOfSunset);
const weatherIcon = chooseWeatherIcon(data)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The icon now comes from the chooseWeatherIcon function (same one that we used for the five day forecast). So now the background img function is only for background img

today.innerHTML = getHeaderHTML(data, timeOfSunrise, timeOfSunset, weatherIcon);
})
.catch((error) => console.error("AAAAAAH!", error))
.finally(() => console.log("YAY!"));
Expand Down