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.

Sign up for GitHub

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

Snow Leopards - Arika A. Ja H. #76

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

Snow Leopards - Arika A. Ja H. #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
64c6505
linked index.js, stylesheet, and axios to html document
lavazane Dec 7, 2022
2c82c66
"added set up stuff"
arikaagnew Dec 7, 2022
1cb834d
"fixed merge conflict"
arikaagnew Dec 7, 2022
183cae8
"updated html"
arikaagnew Dec 7, 2022
9eb8075
take away changes
lavazane Dec 7, 2022
faf5784
added increase and decrease temperature functions
lavazane Dec 8, 2022
ba654a2
"wave 2 complete"
arikaagnew Dec 8, 2022
058d80d
"wave 5 complete"
arikaagnew Dec 9, 2022
2fe96f8
Wave 3: Implement updateCity function
lavazane Dec 9, 2022
62ebc52
fixed spelling
lavazane Dec 9, 2022
7e712c9
wave 5: calling api, not finished
lavazane Dec 10, 2022
a25969a
"started adding styling"
arikaagnew Dec 10, 2022
0555ac7
"merge conflict"
arikaagnew Dec 10, 2022
381facd
"completed all waves"
arikaagnew Dec 10, 2022
b1774a6
add update color to getweather function
lavazane Dec 10, 2022
bb186cb
add updateLanscape function to getWeather in order for weather garden…
lavazane Dec 12, 2022
2c3bb1c
added container class
lavazane Dec 12, 2022
e0bdee9
added styling
lavazane Dec 12, 2022
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
47 changes: 46 additions & 1 deletion index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,57 @@

Copy link

Choose a reason for hiding this comment

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

Be sure to validate your html when possible. The errors/warnings can help us write cleaner HTML. Even if our markup looks fine in the browser we're using, there's no guarantee invalid HTML will do so on other browsers, and it could even break in the same browser since there are no layout guarantees made for invalid HTML.

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Weather Report</title>
<link href="styles/index.css" rel="stylesheet">
</head>
<body>

<header>
<h1 id="weatherHeader">Weather Report</h1>
<h2> For the City of <span id="cityNameHeader">Seattle</span></h2>
</header>
<section id ="websiteContainer">
<section class = "container" id="temperatureContainer">
<h2 id="changeTemperature"> Temperature</h2>

<button id="increaseButton">⬆️</button>

<p id="startingTemp">
60
Copy link

Choose a reason for hiding this comment

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

Rather than hard-coding a value for UI elements (which just happens to match the initial value backing the control), I like to leave the initial markup blank, and update the UI on first load from the initial backing values in the JS code. So like here, leave off the 60, then when the document loads, update the UI in the JS to show the initial temperature value. This would protect us from changing the default starting JS value, but having it be out of sync with what the UI is showing.

I would tend to do this for any of the configurable elements (temperature, ground, sky, city).

</p>

<button id="decreaseButton">⬇️</button>

<button id="getTempButton">Get Real Time Temperature</button>
</section>
<section class = "container" id="skyContainer">
<h2 id="skyView"> Sky</h2>

<select id="skyChoice">
<option value="rainy">Rainy</option>
<option value="sunny">Sunny</option>
<option value="cloudy">Cloudy</option>
<option value="snowy">Snowy</option>
</select>
</section>
<section class="container" id="cityBox">
<h2 id="cityName"> City Name</h2>
<form class="example">
<input type="text" id="inputCity" placeholder="">
<button type="submit"><i class="fa fa-search"></i>Reset</button>
Copy link

Choose a reason for hiding this comment

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

It looks like you were planning to bring in Font Awesome for some icons, but I don't see the needed links/imports.

</form>
</section>
<section class = "container"id = "weatherContainer">
Copy link

Choose a reason for hiding this comment

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

Missing space between the attributes. Prefer to omit spaecs around the = for html attributes.

        <section class="container" id="weatherContainer">

<h2 id="weatherGarden">Weather Garden</h2>
<p id="skyIcons">🌧🌈⛈🌧🌧💧⛈🌧🌦🌧💧🌧🌧</p>
<p id="landscapeIcons">🌾🌾_🍃_🪨__🛤_🌾🌾🌾_🍃</p>
</section>
</section>

<script src="./node_modules/axios/dist/axios.min.js"></script>
<script src="src/index.js"></script>
</body>
</html>
141 changes: 141 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
"use strict";

let state = {
startingTemp : 60,
};

const updateTemperature = state => {
const temperatureContainer = document.getElementById("startingTemp");
Copy link

Choose a reason for hiding this comment

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

Rather than doing a lookup for an element each time it's used, consider looking up all the elements you'll need one when the page loads, and store them for later use. Each time we write getElementById("someElementId") that's an opportunity for a typo.

temperatureContainer.textContent = state.startingTemp;
};

const increaseTemperature = () => {
state.startingTemp += 1;
updateTemperature(state);
updateColor();
updateLandscape();
};

const decreaseTemperature = () => {
state.startingTemp -= 1;
updateTemperature(state);
updateColor();
updateLandscape();
};

const updateColor = () => {
Copy link

Choose a reason for hiding this comment

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

Consider splitting the body of this function into a helper that gets the color information based on the temperature, and another that updates the UI using that value.


if (state.startingTemp >= 80) {
Copy link

Choose a reason for hiding this comment

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

Notice the repetition in these if/else if blocks. Code like this tends to be finicky, since humans tend to make easily overlooked typos that can be hard to track down. Consider a data structure and accompanying code similar to the following:

const tempColors = [
  [80, 'red'],
  [70, 'orange'],
  [60, 'yellow'],
  [50, 'green'],
  [null, 'blue'],
];

const getColorForTemp = (temp) => {
  for (const [boundaryTemp, color] of tempColors) {
    if (boundaryTemp === null || temp >= boundaryTemp) {
      return color;
    }
  }
};

Looking for repetition in the structure of our code and refactoring it to be captured in a data structure instead can make our code more flexible (behavior can be changed solely by changing data) while simplifying the code working with the data.

document.getElementById('startingTemp').style.color = 'red';
Copy link

Choose a reason for hiding this comment

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

Rather than setting part of the style explicitly, prefer to set the css class. The class rule can then affect the color, as well as any other style properties needed. And we could update it by adjusting our style sheet without needing to change any code.

} else if (state.startingTemp >= 70) {
document.getElementById('startingTemp').style.color = 'orange';
} else if (state.startingTemp >= 60) {
document.getElementById('startingTemp').style.color = 'yellow';
} else if (state.startingTemp >= 50) {
document.getElementById('startingTemp').style.color = 'green';
} else if (state.startingTemp < 50) {
document.getElementById('startingTemp').style.color = 'teal';
}
};

const updateLandscape = () => {
Copy link

Choose a reason for hiding this comment

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

Similar considerations as mentioned for the temp/color function would apply to the temp/landscape function.


if (state.startingTemp >= 80) {
document.getElementById('landscapeIcons').textContent = '🌵__🐍_🦂_🌵🌵__🐍_🏜_🦂';
} else if (state.startingTemp >= 70) {
document.getElementById('landscapeIcons').textContent = '🌸🌿🌼__🌷🌻🌿_☘️🌱_🌻🌷';
} else if (state.startingTemp >= 60) {
document.getElementById('landscapeIcons').textContent = '🌾🌾_🍃_🪨__🛤_🌾🌾🌾_🍃';
} else if (state.startingTemp < 60) {
document.getElementById('landscapeIcons').textContent = '🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲';
}
};

const updateSky = () => {
Copy link

Choose a reason for hiding this comment

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

The comments about splitting the function responsibility for temp/color would also apply to the sky function here. To think about how we could make this more data-driven, consider what changes we might make to this function if we had a data structure resembling:

const skySettings = {
  rainy: 'rainy emoji',
  sunny: 'sunny emoji',
  cloudy: 'cloudy emoji',
  snowy: 'snowy emoji',
};


const selectSky = document.getElementById('skyChoice')

if (selectSky.value === 'rainy') {
document.getElementById('skyIcons').textContent = '🌧🌈⛈🌧🌧💧⛈🌧🌦🌧💧🌧';
} else if (selectSky.value === 'sunny') {
document.getElementById('skyIcons').textContent = '☁️ ☁️ ☁️ ☀️ ☁️ ☁️';
} else if (selectSky.value === 'cloudy') {
document.getElementById('skyIcons').textContent = '☁️☁️ ☁️ ☁️☁️ ☁️ 🌤 ☁️ ☁️☁️';
} else if (selectSky.value === 'snowy') {
document.getElementById('skyIcons').textContent = '🌨❄️🌨🌨❄️❄️🌨❄️🌨❄️❄️🌨🌨';
}
};

const updateCity = () => {
Copy link

Choose a reason for hiding this comment

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

This is registered as an event handler, meaning the browser is going to try to pass event details in as the first parameter. So if we added a parameter, maybe called event, we could access the value of the input as event.target.value

Copy link

Choose a reason for hiding this comment

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

Rather than looking directly at the value of the city input to get the value, consider creating an app variable to store the current city name, and then update the UI from that variable. So when the input raises an event, we could update the state variable, and then refresh the state UI (similar to refreshing the temperature UI). This separates the responsibilities of user interaction (the event handler), storing the current application state (the new variable), and updating the UI to reflect the current application state.

const inputCityName = document.getElementById("inputCity");
const cityNameInHeader = document.getElementById("cityNameHeader");
cityNameInHeader.textContent = inputCityName.value;

};


const getLongLat = () => {
let latitude, longitude;
axios.get('http://127.0.0.1:5000/location',
Copy link

Choose a reason for hiding this comment

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

Consider creating a global variable to hold the server part of the address you're contacting (the base URL), then use that variable in your API call addresses (such as by using template strings). If you need to point your endpoints elsewhere (such as when deploying), this can make it much easier to ensure everything is updated together.

Copy link

Choose a reason for hiding this comment

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

We should always return any promise chains we create in helper functions (axios.get starts a promise chain) so that additional actions could be chained onto the end after the function logic completes.

  return axios.get(...).then(...)

{
params: {
q: document.getElementById("cityNameHeader").textContent
Copy link

Choose a reason for hiding this comment

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

Prefer not grabbing values out of the UI for further processing. Rather, have a variable that's used both to update the UI display, and can be referenced to perform related operations (like getting the weather for the city).

}
})
.then((response) => {
latitude = response.data[0].lat;
longitude = response.data[0].lon;

getWeather(latitude, longitude);
Copy link

Choose a reason for hiding this comment

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

Rather then calling the weather api call directly, consider returning the lat/lon from this .then callback so that it would be available to act as the input to a subsequent .then callback. We could then make a separate function whose responsibility is to combine the city->lat/lon function with the lat/lon->weather function.

Assuming getLongLat was a function taking a city name and returning a promise to an object shaped like {lat: 0, lon: 0}, and getWeather was a function taking lat and lon values, and returning a promise to a temperature, we could write a third function getWeatherForCity as follows:

const getWeatherForCity = (cityName) => {
  return getLongLat(cityName)
    .then(({lat, lon}) => {
      return getWeather(lat, lon);
    })
};

We could then use this as

getWeatherForCity(state.city)
  .then(temp => {
      // code currently in getWeather below
      document.getElementById('startingTemp').textContent = Math.round(fahrenheit);
      state.startingTemp = Math.round(fahrenheit);
      updateColor();
      updateLandscape();
  })

})
.catch( (error) => {
console.log("error in finding latitude and longitude");
});
}
const getWeather = (latitude, longitude) => {
axios.get('http://127.0.0.1:5000/weather',
Copy link

Choose a reason for hiding this comment

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

Be sure to return here as mentioned above.

{
params: {
lat: latitude,
lon : longitude,
}
})
.then( (response) => {
const temperature = response.data.main.temp;
const fahrenheit = 1.8*(temperature-273) + 32
document.getElementById('startingTemp').textContent = Math.round(fahrenheit);
state.startingTemp = Math.round(fahrenheit);
updateColor();
updateLandscape();
Comment on lines +106 to +109
Copy link

Choose a reason for hiding this comment

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

Rather than updating the application immediately here, consider returning the temperature from the .then callback. This would make it available to a subsequent .then callback that could be chained outside this helper. In that case, we also need to be sure to return the end of the promise chain itself from the helper, so that additional actions can be chained to run after this helper completes.

})
.catch ((error) => {
console.log('error in finding weather');
});

}

const registerEventHandlers = () => {
const increaseTemperatureButton = document.getElementById('increaseButton');
Copy link

Choose a reason for hiding this comment

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

Consider either storing these element references in your state for later use, or even making global variables to hold them, along with any other elements needed for the operation of the app.

increaseTemperatureButton.addEventListener("click", increaseTemperature);

const decreaseTemperatureButton = document.getElementById('decreaseButton');
decreaseTemperatureButton.addEventListener("click", decreaseTemperature);

const selectElement = document.getElementById('skyChoice');
selectElement.addEventListener('change',updateSky);

const userInputCity = document.getElementById('inputCity');
userInputCity.addEventListener('input', updateCity);

const getRealTimeTemperature = document.getElementById('getTempButton');
getRealTimeTemperature.addEventListener('click', getLongLat);

};

document.addEventListener("DOMContentLoaded", registerEventHandlers);






110 changes: 110 additions & 0 deletions styles/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@

body {
font-family: 'Verdana', sans-serif, monospace;
background-color: rgb(122, 172, 218);
margin: 10px;
text-align: center;
}

button:hover {
background-color: rgb(138, 138, 138);
}

#websiteContainer {
display: grid;
Copy link

Choose a reason for hiding this comment

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

Nice use of grid to layout your page.

grid-template-columns: 1fr 1fr 1fr 1fr;
grid-template-rows: 1fr 1fr 1fr ;
margin: 20px;
}

.container {
width: 300px;
padding: 50px;
margin: 20px;
border-radius: 25px;
text-align: center;
padding-top: 0px;
background-color: white;
border: 15px solid rgb(255, 255, 255);
}

#cityBox {
grid-column: 1 / 2;
grid-row: 3 / 4;
}

#weatherContainer {
grid-column: 3 / 4;
grid-row: 2 / 3;
background-color: white;
width: 400px;
padding: 40px;
padding-top: 0px;
position: relative;
}

#temperatureContainer {
grid-column: 1 / 2;
grid-row: 1 / 2;
}

#skyContainer {
grid-column: 1 / 2;
grid-row: 2 / 3;
}


#cityNameHeader {
font-style: italic;
font-size: 24px;
}

header h2 {
font-size: 20px;
}

section h2 {
font-size: 30px;
}

#getTempButton {
background-color: lightpink;
color: white;
padding: 15px 32px;
font-size: 16px;
border: none;
margin: 10px;
border-radius: 15px;
}

#getTempButton:hover {
background-color: rgb(158, 158, 158);
}

#increaseButton {
font-size: x-large;
}

#decreaseButton {
font-size: x-large;

}

#startingTemp {
font-size: 50px;
font-weight: bold;
margin: 5px;
color: yellow;
}

#skyIcons {
font-size: 30px;
position:absolute;
Copy link

Choose a reason for hiding this comment

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

Some other approaches to avoid going down the road of positioning (this way lies madness) could be to use a grid for the contents of the weatherContainer and give all the growth to a middle row, leaving minimally sized rows to hold the sky and ground.

As it is, the absolute setting here is modified by this element being nested in a relative element higher up, which becomes the baseline against which child absolute positioning takes place.

Some layouts are absolutely achieved most directly using position rules, but if we have other layout methods, it can leave things open to future modifications if we avoid them. My personal feeling is that position rules tend to be very finicky and prone to breakage, though that may just be me.

top: 15%;
}

#landscapeIcons {
font-size: 30px;
position: absolute;
bottom: 0;
}