-
Notifications
You must be signed in to change notification settings - Fork 24
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
C22 Sphinx - Aleida Vieyra + Vlada Rapaport #21
base: main
Are you sure you want to change the base?
Changes from all commits
4538a2f
ad3e5f5
72b8dc0
f06ac49
473ea5d
68270a7
a90db74
27ca974
e551834
02e81f8
6e10aaa
2e1b0b8
809ec89
25a4554
d4fd177
9ca8f91
4a4e399
00dcbf2
efe4716
1a4d420
b1bd35e
b972a78
8426b8e
36887dd
323d8d4
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,15 +1,60 @@ | ||
<!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> | ||
<header class="header__header"> | ||
<h1>Weather Report</h1> | ||
<span>For the lovely city of | ||
<span id="headerCityName" class="header__city-name"> Seattle </span></span> | ||
</header> | ||
<section class="temperature__section"> | ||
<h2>Temperature (F)</h2> | ||
<!-- Add element to display temperature --> | ||
<div class="temperature__content"> | ||
<div class="temperature__controls"> | ||
<span id="increaseTempControl">⬆️</span> | ||
<span id="tempValue"></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"> | ||
<!-- sky options here --> | ||
<option value="select sky option">Select Sky Option</option> | ||
<option value="sunny">Sunny</option> | ||
<option value="cloudy">Cloudy</option> | ||
<option value="rainy">Rainy</option> | ||
<option value="snowy">Snowy</option> | ||
</select> | ||
</section> | ||
|
||
<section class="city-name__section"> | ||
<h2>City Name</h2> | ||
<input type="text" id="cityNameInput" value="Seattle"> | ||
<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="https://unpkg.com/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,223 @@ | ||
const state = { | ||
currentTemp: 0, | ||
cityName: "Miami", | ||
defaultCityName: "Seattle", | ||
}; | ||
Comment on lines
+1
to
+5
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. Nice work on selecting what pieces of data should go in state! I think that these a the quintessential choices! ✨ |
||
|
||
/************************/ | ||
/******* Wave 2 *********/ | ||
/************************/ | ||
|
||
// Function to update the temperature display and apply color changes | ||
const updateTemperatureDisplay = () => { | ||
const tempValueElement = document.getElementById("tempValue"); | ||
const landscapeElement = document.getElementById("landscape"); | ||
tempValueElement.textContent = `${state.currentTemp}°`; | ||
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. I want to say that this could just be of the type |
||
|
||
if (state.currentTemp >= 80) { | ||
tempValueElement.style.color = "red"; | ||
landscapeElement.textContent = "🌵 🐍 🦂 🌵🌵 🐍 🏜 🦂"; | ||
} else if (state.currentTemp >= 70) { | ||
tempValueElement.style.color = "orange"; | ||
landscapeElement.textContent = "🌸🌿🌼 🌷🌻🌿 ☘️🌱 🌻🌷"; | ||
} else if (state.currentTemp >= 60) { | ||
tempValueElement.style.color = "salmon"; | ||
landscapeElement.textContent = "🌾🌾 🍃 🪨 🛤 🌾🌾🌾 🍃"; | ||
} else if (state.currentTemp >= 50) { | ||
tempValueElement.style.color = "green"; | ||
landscapeElement.textContent = "🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲"; | ||
} else if (state.currentTemp <= 49) { | ||
tempValueElement.style.color = "blue"; | ||
landscapeElement.textContent = "⛄️ ⛄️ ⛄️"; | ||
} | ||
Comment on lines
+17
to
+32
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. To make this a little more concise and avoid the multiple if statements we could use a loop and a list to do something like this instead: 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
}
} 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? |
||
}; | ||
|
||
// Function to increase temperature | ||
const increaseTemp = () => { | ||
state.currentTemp += 1; | ||
updateTemperatureDisplay(); | ||
}; | ||
|
||
// Function to decrease temperature | ||
const decreaseTemp = () => { | ||
state.currentTemp -= 1; | ||
updateTemperatureDisplay(); | ||
}; | ||
Comment on lines
+36
to
+45
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. ⭐️ |
||
|
||
/** Wave 2 events **/ | ||
const temperatureControls = () => { | ||
const warmUpButton = document.getElementById("increaseTempControl"); | ||
const coolDownButton = document.getElementById("decreaseTempControl"); | ||
|
||
warmUpButton.addEventListener("click", increaseTemp); | ||
coolDownButton.addEventListener("click", decreaseTemp); | ||
}; | ||
Comment on lines
+48
to
+54
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. Straightforward and correct, no notes! |
||
|
||
|
||
/************************/ | ||
/******* Wave 3 *********/ | ||
/************************/ | ||
|
||
const updateCity = () => { | ||
const cityNameInput = document.getElementById("cityNameInput"); | ||
const currentCityHeader = document.getElementById("headerCityName"); | ||
currentCityHeader.textContent = cityNameInput.value; | ||
}; | ||
|
||
const handleUpdateCityEvent = () => { | ||
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. I wonder if this function name is accurately descriptive of what the function is doing. To me it's not really handling the the event. More so it is grabbing the element we wish to add an event to. |
||
const cityNameInput = document.getElementById("cityNameInput"); | ||
const resetButton = document.getElementById("cityNameReset"); | ||
cityNameInput.addEventListener("input", updateCity); | ||
resetButton.addEventListener("click", ResetCity); | ||
Comment on lines
+68
to
+71
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. Looks like we are doing similar logic in a few places (finding an element by |
||
}; | ||
|
||
/************************/ | ||
/******* Wave 4 *********/ | ||
/************************/ | ||
|
||
const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); | ||
|
||
// Event handler function to retrieve user input: | ||
const retrieveCityInput = () => { | ||
const cityNameInput = document.getElementById("cityNameInput"); | ||
return cityNameInput.value; | ||
}; | ||
|
||
// API Call to LocationIQ for location coordinates | ||
const getCoordinates = () => { | ||
let location = retrieveCityInput(); | ||
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. Doesn't seem like this variable is being changed anywhere else in this function. Could it be initialized with the |
||
|
||
return axios | ||
.get("http://127.0.0.1:5000/location", { | ||
params: { | ||
q: location, | ||
}, | ||
}) | ||
.then((response) => { | ||
let latitude = response.data[0].lat; | ||
let longitude = response.data[0].lon; | ||
|
||
console.log( | ||
`Location: ${location}, Latitude: ${latitude}, Longitude: ${longitude}` | ||
); | ||
Comment on lines
+100
to
+102
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 forget to remove console logs like this, you could accidentally leak private data. Also just helps to keep the code clutter free. |
||
return { | ||
latitude: latitude, | ||
longitude: longitude, | ||
}; | ||
}) | ||
Comment on lines
+90
to
+107
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. Great work on this promise! Very easy to follow! Especially since you also decided to use different helper functions to tie things together! |
||
.catch((error) => { | ||
console.log("Error found inside getCoordinates function!"); | ||
console.log( | ||
`The value of status inside of error response is: | ||
${error.response.status}` | ||
); | ||
}); | ||
}; | ||
|
||
// API Call for weather status/temp of current city displayed | ||
const getCurrentCityWeather = () => { | ||
// Info from OpenWeather API Call documentation: | ||
// https://openweathermap.org/current#geo | ||
return getCoordinates() | ||
.then((coordinatesResponse) => { | ||
if (!coordinatesResponse) { | ||
throw new Error("Coordinates could not be retrieved."); | ||
Comment on lines
+123
to
+124
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. You could also throw this error in the catch above so that the error doesn't propagate further down like it does here. |
||
} else { | ||
return axios.get("http://127.0.0.1:5000/weather", { | ||
params: { | ||
lat: coordinatesResponse.latitude, | ||
lon: coordinatesResponse.longitude, | ||
}, | ||
}); | ||
} | ||
}) | ||
.then((locationTempResponse) => { | ||
let temp = locationTempResponse.data.main.temp; // in KELVIN | ||
console.log(`Current Location temperature is: ${temp}`); | ||
return temp; | ||
}) | ||
.catch((error) => { | ||
console.log("Error found inside getCurrentCityWeather!"); | ||
console.log( | ||
`The value of status inside of error response is: | ||
${error}` | ||
); | ||
}); | ||
}; | ||
|
||
const updateRealtimeTempBtn = () => { | ||
// Chaining the axios Promise API call/response from above | ||
getCurrentCityWeather().then((tempKelvin) => { | ||
if (tempKelvin) { | ||
const conversion = (tempKelvin - 273) * (9 / 5) + 32; | ||
state.currentTemp = Math.round(conversion); | ||
updateTemperatureDisplay(); | ||
} | ||
}); | ||
Comment on lines
+150
to
+156
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. Nice chaining! Only suggestion, moving the temperature conversion logic into its own function so that it would be more readable since we are already working inside of a |
||
}; | ||
|
||
const getRealtimeTempButton = () => { | ||
const realtimeTemp = document.getElementById("currentTempButton"); | ||
realtimeTemp.addEventListener("click", updateRealtimeTempBtn); | ||
}; | ||
|
||
/************************/ | ||
/******* Wave 5 *********/ | ||
/************************/ | ||
|
||
const skyOptions = { | ||
sunny: "☁️ ☁️ ☁️ ☀️ ☁️ ☁️", | ||
cloudy: "☁️☁️ ☁️ ☁️☁️ ☁️ 🌤 ☁️ ☁️☁️", | ||
rainy: "🌧🌈⛈🌧🌧💧⛈🌧🌦🌧💧🌧🌧", | ||
snowy: "🌨❄️🌨🌨❄️❄️🌨❄️🌨❄️❄️🌨🌨", | ||
}; | ||
|
||
// Function to update the sky display | ||
const updateSky = () => { | ||
const skySelect = document.getElementById("skySelect"); | ||
const skyDisplay = document.getElementById("sky"); | ||
|
||
// Get the selected option value | ||
const selectedSky = skySelect.value; | ||
|
||
// Update the sky display | ||
skyDisplay.textContent = skyOptions[selectedSky]; | ||
skySelect.addEventListener("change", updateSky); | ||
}; | ||
|
||
/************************/ | ||
/******* Wave 6 *********/ | ||
/************************/ | ||
|
||
// reset city name to default | ||
// Event handler function to UPDATE the headerCityName to user's input: | ||
const ResetCity = () => { | ||
const currentCityHeader = document.getElementById("headerCityName"); | ||
currentCityHeader.textContent = state.defaultCityName; | ||
|
||
const cityNameInput = document.getElementById("cityNameInput"); | ||
cityNameInput.value = state.defaultCityName; | ||
}; | ||
|
||
|
||
/************************/ | ||
/**** Event Handling ****/ | ||
/************************/ | ||
|
||
const registerEventHandlers = () => { | ||
/** Wave 2 events **/ | ||
temperatureControls(); | ||
|
||
/** Wave 3 events **/ | ||
handleUpdateCityEvent(); | ||
|
||
/**** Wave 5 events ****/ | ||
updateSky(); | ||
|
||
|
||
getRealtimeTempButton(); | ||
updateTemperatureDisplay(); | ||
}; | ||
|
||
// Initialize the application when DOM content is loaded | ||
document.addEventListener("DOMContentLoaded", registerEventHandlers); |
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.
The spacing is being used for readability and separating different parts of the page, this is fine for me. Just be mindful that some developers might prefer for there to be no empty spaces.