-
Notifications
You must be signed in to change notification settings - Fork 85
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
Gwen and Whitney - Zoisite #56
base: main
Are you sure you want to change the base?
Changes from all commits
b7b7e35
3e23f6c
e5434df
fb927f2
041b397
bb2e031
d82991a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,58 @@ | ||
<!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 rel="preconnect" href="https://fonts.gstatic.com"> | ||
<link href="https://fonts.googleapis.com/css2?family=Rubik&display=swap" rel="stylesheet"> | ||
<link rel="stylesheet" href="styles/index.css" /> | ||
<link rel="stylesheet" href="styles/index.css"> | ||
</head> | ||
|
||
<body> | ||
<script src="./src/index.js"></script> | ||
<header class="header__header"> | ||
<h1>Weather Report</h1> | ||
<span>For the lovely city of | ||
<span id="headerCityName" class="header__city-name"></span></span> | ||
</header> | ||
<section class="temperature__section"> | ||
<h2>Temperature</h2> | ||
<div class="temperature__content"> | ||
<div class="temperature__controls"> | ||
<span id="increaseTempControl">⬆️</span> | ||
<span id="tempValue">0</span> | ||
<span id="decreaseTempControl">⬇️</span> | ||
</div> | ||
<button id="currentTempButton">Get Realtime Temperature</button> | ||
</div> | ||
</section> | ||
<section class="sky__section"> | ||
<h2>Sky</h2> | ||
<select id="skySelect"> | ||
<option value="sunny">☁️ ☁️ ☁️ ☀️ ☁️ ☁️</option> | ||
<option value="cloudy">☁️☁️ ☁️ ☁️☁️ ☁️ 🌤 ☁️ ☁️☁️</option> | ||
<option value="rainy">🌧🌈⛈🌧🌧💧⛈🌧🌦🌧💧🌧🌧</option> | ||
<option value="snowy">🌨❄️🌨🌨❄️❄️🌨❄️🌨❄️❄️🌨🌨</option> | ||
<!-- sky options here --> | ||
</select> | ||
</section> | ||
|
||
<section class="city-name__section"> | ||
<h2>City Name</h2> | ||
<input type="text" id="cityNameInput"> | ||
<button id="cityNameReset" class="city-name__reset-btn">Reset</button> | ||
</section> | ||
|
||
<section class="garden__section"> | ||
<h2>Weather Garden</h2> | ||
<div id="gardenContent" class="garden__content"> | ||
<div id="sky"></div> | ||
<div id="landscape">🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲</div> | ||
</div> | ||
</section> | ||
<script src="./node_modules/axios/dist/axios.min.js"></script> | ||
<script src="./src/index.js"></script> | ||
</body> | ||
</html> | ||
</html> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
|
||
const state = { | ||
tempDeg: 0, | ||
defaultColor: "teal", | ||
cityName : "Seattle", | ||
defaultLandscape: "🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲", | ||
hotLandscape: "🌵__🐍_🦂_🌵🌵__🐍_🏜_🦂", | ||
niceLandscape: "🌸🌿🌼__🌷🌻🌿_☘️🌱_🌻🌷", | ||
midLandscape: "🌾🌾_🍃_🪨__🛤_🌾🌾🌾_🍃", | ||
increaseTempButton: null, | ||
decreaseTempButton: null, | ||
tempValue: null, | ||
landscape: null, | ||
cityNameOutput : null, | ||
cityNameInput: null, | ||
currentWeatherButton : null, | ||
resetButton : null, | ||
skySelect : null, | ||
sky : null, | ||
}; | ||
|
||
const setDefaultValues = () => { | ||
state.cityNameInput.value = `${state.cityName}`; | ||
state.cityNameOutput.textContent = `${state.cityName}`; | ||
state.landscape.textContent = `${state.defaultLandscape}`; | ||
tempValue.style.color = state.defaultColor; | ||
tempValue.textContent = 0; | ||
sky.textContent = ""; | ||
}; | ||
|
||
const registerEventHandlers = () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
state.increaseTempButton.addEventListener("click", increaseTemp, changeColor, changeLandscape); | ||
state.decreaseTempButton.addEventListener("click", decreaseTemp, changeColor, changeLandscape); | ||
state.currentWeatherButton.addEventListener("click", displayCurrentWeather); | ||
state.cityNameInput.addEventListener("input", displayCityName); | ||
state.resetButton.addEventListener("click", setDefaultValues); | ||
state.skySelect.addEventListener("change", changeSky); | ||
}; | ||
|
||
const loadControls = () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
state.increaseTempButton = document.getElementById("increaseTempControl"); | ||
state.decreaseTempButton = document.getElementById("decreaseTempControl"); | ||
state.cityNameInput = document.getElementById("cityNameInput"); | ||
state.cityNameOutput = document.getElementById("headerCityName"); | ||
state.currentWeatherButton = document.getElementById("currentTempButton"); | ||
state.tempValue = document.getElementById("tempValue"); | ||
state.landscape = document.getElementById("landscape"); | ||
state.resetButton = document.getElementById("cityNameReset"); | ||
state.skySelect = document.getElementById("skySelect"); | ||
state.sky = document.getElementById("sky"); | ||
}; | ||
|
||
const increaseTemp = () => { | ||
state.tempDeg += 1; | ||
tempValue.textContent = `${state.tempDeg}` | ||
changeColor(); | ||
changeLandscape(); | ||
}; | ||
|
||
const decreaseTemp = () => { | ||
state.tempDeg -= 1; | ||
tempValue.textContent = `${state.tempDeg}`; | ||
changeColor(); | ||
changeLandscape(); | ||
}; | ||
Comment on lines
+53
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Notice that these two methods do essentially the same thing but the difference is -= 1 or += 1 (also in JS you can use ++ or -- which is short hand for doing the same thing). How could you combine the logic of these two methods into one event handler called handleTempChange()? What if you pass in the event object as an argument and you check if the event's target has an id const handleTempChange = (event) => {
if (event.target.id === 'increaseTempControl') {
state.tempDeg++;
} else {
state.tempDeg--;
}
tempValue.textContent = `${state.tempDeg}`;
changeColor();
changeLandscape();
}; |
||
|
||
const changeColor = () => { | ||
let currentTemp = tempValue.textContent; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use const |
||
if (currentTemp >= 80) { | ||
tempValue.style.color = "red"; | ||
} else if (currentTemp >= 70) { | ||
tempValue.style.color = "orange"; | ||
} else if (currentTemp >= 60) { | ||
tempValue.style.color = "yellow"; | ||
} else if (currentTemp >= 50) { | ||
tempValue.style.color = "green"; | ||
} else { | ||
tempValue.style.color = "teal"; | ||
} | ||
}; | ||
|
||
const changeLandscape = () => { | ||
let currentTemp = tempValue.textContent; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. currentTemp isn't reassigned so we should use const |
||
if (currentTemp >= 80) { | ||
landscape.textContent = `${state.hotLandscape}`; | ||
} else if (currentTemp >= 70) { | ||
landscape.textContent = `${state.niceLandscape}`; | ||
} else if (currentTemp >= 60) { | ||
landscape.textContent = `${state.midLandscape}`; | ||
} else { | ||
landscape.textContent = `${state.defaultLandscape}`; | ||
} | ||
}; | ||
|
||
Comment on lines
+67
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another way to format these two methods would be to use an object. The keys of the object could be temp values and then you could have values be colors (for const colorObject = {80: 'red', 70: 'orange', 60: 'yellow'} // etc When you have an object, you don't need to do if/else if/else if/ else which can introduce bugs and also adds complexity to the method. If you have an object, then you can iterate over the object and check the keys with currentTemp and then get the correct values to set the color and landscape emojis. Also, on lines 85-91, you shouldn't need to use string templates to get the values from the state object. You should be able to just do something like |
||
const displayCityName = (event) => { | ||
const cityInput = event.target.value; | ||
state.cityNameOutput.textContent = cityInput; | ||
}; | ||
|
||
const changeSky = () => { | ||
sky.textContent = `${skySelect.options[skySelect.selectedIndex].text}`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need string templating to get the value of the selected sky. You'd use string templating for concatenating strings, like: const baseUrl = "localhost:500";
axios.get(`${baseUrl}/cats`) |
||
} | ||
|
||
const displayCurrentWeather = () => { | ||
findLatAndLon(); | ||
} | ||
|
||
const findLatAndLon= (query) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick - there should be a whitespace before = |
||
let latitude, longitude; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if you initialized these variables in your state object? Then on lines 118 and 119, you could do:
|
||
axios.get('http://localhost:5000/location', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer to reference string literals like this url with a descriptively named variable |
||
{ | ||
params: { | ||
q: `${state.cityNameOutput.textContent}`, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need for string templating |
||
format: 'json' | ||
} | ||
}) | ||
.then( (response) => { | ||
latitude = response.data[0].lat; | ||
longitude = response.data[0].lon; | ||
console.log('success in findLatAndLon', latitude, longitude) | ||
|
||
findWeather(latitude, longitude); | ||
}) | ||
.catch( (error) => { | ||
console.log('error in findLatAndLon!'); | ||
}) | ||
Comment on lines
+116
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick - indentation. These lines should be indented one level deeper. |
||
}; | ||
|
||
const findWeather = (latitude, longitude) => { | ||
axios.get('http://localhost:5000/weather', | ||
{ | ||
params: { | ||
format: 'json', | ||
lat: latitude, | ||
lon: longitude | ||
} | ||
}) | ||
Comment on lines
+131
to
+137
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer the object being sent along in the get request to be referenced with a variable to make this line of code more readable. WEATHER_URL = "http://localhost:5000/weather";
request_data = {
format: 'json',
lat: latitude,
lon: longitude
}
axios.get(WEATHER_URL, request_data) |
||
.then( (response) => { | ||
|
||
console.log('success in findWeather!', response.data.main.temp); | ||
state.tempDeg = Math.round((response.data.main.temp - 273.15) * 9/5 + 32); | ||
tempValue.textContent = `${state.tempDeg}` | ||
changeColor(); | ||
changeLandscape(); | ||
}) | ||
.catch( (error) => { | ||
console.log('error in findWeather!'); | ||
}); | ||
Comment on lines
+140
to
+148
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick - indentation needs to be fixed up. While the JS still executes correctly since the language is not dependent on indentation, we should strive for correct indentation for sake of consistency and readability. When you open a PR for review when you're at work be sure to fix up your indentation/spacing and remove debugging print statements. |
||
} | ||
|
||
const onLoaded = () => { | ||
loadControls(); | ||
setDefaultValues(); | ||
registerEventHandlers(); | ||
}; | ||
|
||
onLoaded(); | ||
Comment on lines
+151
to
+157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We had to use the onLoaded approach when we were using codesandbox because by the time the UI had rendered and shown us the code editor for to write code the event called DOMContentLoaded had already fired. The act of rendering the web page makes DOMContentLoaded happen so we couldn't use it as an event to register loadControls(). When you're writing vanilla JS for event handling like, we should call register the function called window.addEventListener("DOMContentLoaded", registerEventHandlers) Then when your page loads You could add |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,164 @@ | ||
h2 { | ||
margin: 0 auto 2rem auto; | ||
} | ||
|
||
body { | ||
display: grid; | ||
grid-template-columns: 1fr 2fr; | ||
grid-template-rows: auto auto auto auto; | ||
grid-gap: 1rem; | ||
|
||
font-family: "Rubik", sans-serif; | ||
font-size: 18px; | ||
background-color: #1b69f9; | ||
margin: 2rem; | ||
} | ||
|
||
.header__header { | ||
color: white; | ||
grid-column: span 3; | ||
display: flex; | ||
align-items: center; | ||
margin: 2rem auto 3rem 0; | ||
} | ||
|
||
.header__header > h1 { | ||
margin-right: 2rem; | ||
font-size: 3em; | ||
} | ||
|
||
.header__city-name { | ||
font-style: oblique; | ||
font-size: 2rem; | ||
} | ||
|
||
.header__city-name::before, | ||
.header__city-name::after { | ||
content: "✨"; | ||
} | ||
|
||
.temperature__section, | ||
.sky__section, | ||
.city-name__section { | ||
border-radius: 8px; | ||
padding: 2rem; | ||
background-color: white; | ||
} | ||
|
||
.temperature__section { | ||
grid-row: 2; | ||
} | ||
|
||
.temperature__section button { | ||
background-color: #1b69f9; | ||
border: none; | ||
color: white; | ||
padding: 15px 32px; | ||
text-align: center; | ||
text-decoration: none; | ||
display: inline-block; | ||
font-size: 16px; | ||
border-radius: 10px | ||
} | ||
|
||
.sky__section { | ||
grid-row: 3; | ||
} | ||
|
||
.city-name__section { | ||
grid-row: 4; | ||
} | ||
|
||
.garden__section { | ||
grid-row: 2 / span 3; | ||
grid-column: 2; | ||
text-align: center; | ||
align-self: center; | ||
} | ||
|
||
.temperature__content { | ||
display: flex; | ||
flex-direction: row; | ||
justify-content: space-around; | ||
/* justify-content: center; */ | ||
} | ||
|
||
#tempValue { | ||
font-size: 3rem; | ||
margin-left: 1.5rem; | ||
/* padding-right: 1rem; */ | ||
/* margin-right: 1.5rem; */ | ||
} | ||
|
||
.temperature__controls { | ||
display: flex; | ||
flex-direction: column; | ||
align-items: center; | ||
} | ||
|
||
.garden__section > h2 { | ||
color: white; | ||
} | ||
|
||
.garden__content { | ||
min-height: 200px; | ||
max-width: fit-content; | ||
margin: auto; | ||
padding: 2rem; | ||
|
||
display: flex; | ||
flex-direction: column; | ||
justify-content: space-between; | ||
|
||
border-radius: 8px; | ||
font-size: 2em; | ||
} | ||
|
||
.city-name__reset-btn { | ||
border: 0; | ||
background-color: #1655cc; | ||
color: white; | ||
border-radius: 8px; | ||
padding: 1rem; | ||
font-family: "Rubik", sans-serif; | ||
} | ||
|
||
.red { | ||
color: red; | ||
} | ||
|
||
.orange { | ||
color: orange; | ||
} | ||
|
||
.yellow { | ||
color: gold; | ||
} | ||
|
||
.yellow-green { | ||
color: yellowgreen; | ||
} | ||
|
||
.green { | ||
color: green; | ||
} | ||
|
||
.teal { | ||
color: teal; | ||
} | ||
|
||
.cloudy { | ||
background-color: lightgrey; | ||
} | ||
|
||
.sunny { | ||
background-color: rgb(221, 255, 255); | ||
} | ||
|
||
.rainy { | ||
background-color: lightblue; | ||
} | ||
|
||
.snowy { | ||
background-color: lightsteelblue; | ||
} |
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.
👍