C22 Olga & Izzy #12
C22 Olga & Izzy #12
Conversation
| const updateDecreaseTempCount = () => { | ||
| --state.tempCount; | ||
| refreshUI() | ||
| updateTempColorAndLandscape(); | ||
| }; | ||
|
|
||
| const updateIncreaseTempCount = () => { | ||
| ++state.tempCount; | ||
| refreshUI(); | ||
| updateTempColorAndLandscape(); | ||
| }; | ||
|
|
||
| const refreshUI = () => { | ||
| state.tempValue.textContent = `${state.tempCount}°F`; | ||
| }; |
There was a problem hiding this comment.
Great work on these functions! The only thing I would change is the refreshUI function name. This name makes think that it would be refreshing the entire page when in actuality it is simply updating the text content for the element displaying the temperature.
| switch (true) { | ||
| case state.tempCount >= 80: | ||
| tempValue.style.color = "red"; | ||
| newLandscape.textContent = "🌵__🐍_🦂_🌵🌵__🐍_🏜_🦂"; | ||
| break; | ||
| case state.tempCount >= 70: | ||
| tempValue.style.color = "orange"; | ||
| newLandscape.textContent = "🌸🌿🌼__🌷🌻🌿_☘️🌱_🌻🌷"; | ||
| break; | ||
| case state.tempCount >= 60: | ||
| tempValue.style.color = "yellow"; | ||
| newLandscape.textContent = "🌾🌾_🍃_🪨__🛤_🌾🌾🌾_🍃"; | ||
| break; | ||
| case state.tempCount >= 50: | ||
| tempValue.style.color = "green"; | ||
| newLandscape.textContent = "🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲"; | ||
| break; | ||
| case state.tempCount < 50: | ||
| tempValue.style.color = "teal"; | ||
| newLandscape.textContent = "🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲"; | ||
| break; |
There was a problem hiding this comment.
Nice using the switch syntax for this! Another way to do this is use a loop and a list to do something like this:
const temperatureRanges = [
{ min: 80, color: "red", landscape: "🌵 🐍 🦂 🌵🌵 🐍 🏜 🦂" },
{ min: 70, color: "orange", landscape: "🌸🌿🌼 🌷🌻🌿 ☘️🌱 🌻🌷" },
{ min: 60, color: "salmon", landscape: "🌾🌾 🍃 🪨 🛤 🌾🌾🌾 🍃" },
{ min: 50, color: "green", landscape: "🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲" },
{ min: -Infinity, color: "blue", landscape: "⛄️ ⛄️ ⛄️" }, // Default for temperatures <= 49
];
for (const range of temperatureRanges) {
if (state.currentTemp >= range.min) {
tempValueElement.style.color = range.color;
landscapeElement.textContent = range.landscape;
break; // Stop once the matching range is found
}
}It helps you avoid all the if statements. You do, however, lose some readability. Mainly because the person would need to realize that it utilizes the order retention of a list. Which one do you think you gravitate to and why?
| const resetCityNameInput = () => { | ||
| cityNameInput.value = ""; | ||
| headerCityName.textContent = ""; | ||
| }; |
There was a problem hiding this comment.
Shouldn't we also update our state object as well? That way we don't have `state saying one thing and the UI displaying another.
|
|
||
| //wave 4 | ||
| const findTemperature = (coordinates) => { | ||
| console.log(coordinates) |
There was a problem hiding this comment.
Don't forget to remove console logs like this, you could accidentally leak private data. Also just helps to keep the code clutter free.
| }) | ||
| .catch((error) => { | ||
| console.log("error in findTemperature!", error); | ||
| throw error; |
There was a problem hiding this comment.
Nice work throwing an error, it makes sure that it doesn't propagate through your following functionality.
| const findLatitudeAndLongitude = () => { | ||
| let latitude, longitude; | ||
| return axios | ||
| .get("http://127.0.0.1:5000/location", { | ||
| params: { | ||
| q: state.cityName, | ||
| format: "json", | ||
| }, | ||
| }) | ||
| .then((response) => { | ||
| latitude = response.data[0].lat; | ||
| longitude = response.data[0].lon; | ||
| console.log("success in findLatitudeAndLongitude!", latitude, longitude); | ||
| return { | ||
| lat: latitude, | ||
| lon: longitude, | ||
| }; | ||
| }) | ||
| .catch((error) => { | ||
| console.log("error in findLatitudeAndLongitude!"); | ||
| throw error; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Great work on this function! Very easy to follow!
| const getTemperature = () => { | ||
|
|
||
| return findLatitudeAndLongitude() | ||
| .then((coordinates) => { | ||
| return findTemperature(coordinates); | ||
| }) | ||
| .then((temp) => { | ||
| const fahrenheit = 1.8 * (temp - 273) + 32; | ||
| state.tempCount = Math.round(fahrenheit); | ||
| refreshUI(); | ||
| updateTempColorAndLandscape(); | ||
| }) | ||
| .catch((error) => { | ||
| console.error("Error in getTemperature!"); | ||
| state.tempValue.textContent = "Enter a valid city" | ||
| }); |
| const updateSky = () => { | ||
| const skySelect = document.getElementById('skySelect').value; | ||
| const skyContainer = document.getElementById('sky'); | ||
|
|
||
| skyContainer.textContent = ""; | ||
|
|
||
| let sky; | ||
| let skyBackgroundClass; | ||
|
|
||
| switch (skySelect) { | ||
| case 'sunny': | ||
| sky = '☀️ ☀️ ☀️ ☀️ ☀️ ☀️'; | ||
| skyBackgroundClass = 'sunny'; | ||
| break; | ||
| case 'cloudy': | ||
| sky = '☁️☁️ ☁️ ☁️☁️ ☁️ 🌤 ☁️ ☁️☁️'; | ||
| skyBackgroundClass = 'cloudy'; | ||
| break; | ||
| case 'rainy': | ||
| sky = '🌧🌈⛈🌧🌧💧⛈🌧🌦🌧💧🌧🌧'; | ||
| skyBackgroundClass = 'rainy'; | ||
| break; | ||
| case 'snowy': | ||
| sky = '🌨❄️🌨🌨❄️❄️🌨❄️🌨❄️❄️🌨🌨'; | ||
| skyBackgroundClass = 'snowy'; | ||
| break; | ||
|
|
||
| } |
There was a problem hiding this comment.
Same thing about the landscape could apply here.
| const registerEventHandlers = () => { | ||
| loadControls(); | ||
|
|
||
| const decreaseTemp = document.getElementById("decreaseTempControl"); | ||
| decreaseTemp.addEventListener("click", updateDecreaseTempCount); | ||
|
|
||
| const increaseTemp = document.getElementById("increaseTempControl"); | ||
| increaseTemp.addEventListener("click", updateIncreaseTempCount); | ||
|
|
||
| const skySelect = document.getElementById('skySelect'); | ||
| skySelect.addEventListener('change', updateSky); | ||
|
|
||
| const cityNameReset = document.getElementById("cityNameReset"); | ||
| cityNameReset.addEventListener("click", resetCityNameInput); | ||
| cityNameInput.addEventListener("input", updateCityNameInput); | ||
|
|
||
| const currentTempButton = document.getElementById("currentTempButton"); | ||
| currentTempButton.addEventListener("click", getTemperature); | ||
|
|
||
| updateTempColorAndLandscape(); | ||
| }; | ||
|
|
||
| document.addEventListener('DOMContentLoaded', registerEventHandlers); |
There was a problem hiding this comment.
Nice work ensuring that your document was loaded because adding all your functionality!
No description provided.