-
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
Phoenix-AnhTran , Sphinx-Tatiana-Snook #7
base: main
Are you sure you want to change the base?
Changes from all commits
e5193fe
2a98557
a5c5594
216b525
f9a2532
2b20710
90c188b
f7bc38e
17c7a14
0d321b3
1909956
4d239c3
012a9f2
0a4cc58
c03f30b
6ade546
32a1e4e
87c5141
5165003
a4132eb
7efa5a3
ec14029
399fb16
4d3b572
fcc3718
aa8c0a9
b445734
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,59 @@ | ||
<!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</h2> | ||
<div class="temperature__content"> | ||
<div class="temperature__controls"> | ||
<span id="increaseTempControl">⬆️</span> | ||
<span id="tempValue"></span> | ||
<span id="decreaseTempControl">⬇️</span> | ||
</div> | ||
<button id="getRealTimeTempButton">Get Realtime Temperature</button> | ||
</div> | ||
|
||
</section> | ||
<section class="sky__section"> | ||
<h2>Sky</h2> | ||
<select id="skySelect"> | ||
<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"> | ||
<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="skyDisplay">☁️ ☁️ ☁️ ☀️ ☁️ ☁️</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,145 @@ | ||
let temperature = 48; | ||
|
||
const tempValue = document.getElementById('tempValue'); | ||
Comment on lines
+1
to
+3
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 appropriately using the |
||
const increaseTempControl = document.getElementById('increaseTempControl'); | ||
const decreaseTempControl = document.getElementById('decreaseTempControl'); | ||
const landScape = document.getElementById('landscape'); | ||
const cityNameInput = document.getElementById('cityNameInput'); | ||
const cityNameReset = document.getElementById('cityNameReset'); | ||
const headerCityName = document.getElementById('headerCityName'); | ||
const getRealTimeTempButton = document.getElementById('getRealTimeTempButton'); | ||
Comment on lines
+4
to
+10
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 |
||
|
||
//Update the sky options when we choose different sky and change the color background in garden | ||
const skyOptions = { | ||
'sunny': { | ||
'text': "☁️ ☁️ ☁️ ☀️ ☁️ ☁️", | ||
'backgroundColor': "#DFFFFF", | ||
}, | ||
'cloudy': { | ||
'text': "☁️☁️ ☁️ ☁️☁️ ☁️ 🌤 ☁️ ☁️☁️", | ||
'backgroundColor': "#D3D3D3", | ||
}, | ||
'rainy': { | ||
'text': "🌧🌈⛈🌧🌧💧⛈🌧🌦🌧💧🌧🌧", | ||
'backgroundColor': "#B0C3DF", | ||
}, | ||
'snowy': { | ||
'text': "🌨❄️🌨🌨❄️❄️🌨❄️🌨❄️❄️🌨🌨", | ||
'backgroundColor': "#ADD9E6", | ||
}, | ||
}; | ||
Comment on lines
+13
to
+30
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 way of storing these! You could even put them in another file and just import them to better declutter your code! |
||
const skySelect = document.getElementById("skySelect"); | ||
const skyDisplay = document.getElementById("skyDisplay") | ||
const gardenContent = document.getElementById("gardenContent") | ||
|
||
skySelect.addEventListener("change", (event) => { | ||
const skySelected = event.target.value; | ||
console.log("Selected sky:", skySelected); | ||
updateSky(skySelected); | ||
}); | ||
Comment on lines
+31
to
+39
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. A few things here:
|
||
|
||
const updateSky = (sky) => { | ||
const skySelectOption = skyOptions[sky] | ||
console.log(skySelectOption) | ||
if (skySelectOption) { | ||
skyDisplay.textContent = skySelectOption.text; | ||
gardenContent.style.backgroundColor = skySelectOption.backgroundColor; | ||
} | ||
}; | ||
|
||
//Update the temperature and change font color and landscape when increase and decrease temperature | ||
const updateTemperature = () => { | ||
tempValue.textContent = `${temperature}°F`; | ||
tempValue.style.color = getFontColor(temperature); | ||
landScape.textContent = getLandscape(temperature); | ||
}; | ||
|
||
const getLandscape = (temp) => { | ||
if (temp <= 59) return '🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲'; | ||
if (temp <= 69) return '🌾🌾_🍃_🪨__🛤_🌾🌾🌾_🍃'; | ||
if (temp <= 79) return '🌸🌿🌼__🌷🌻🌿_☘️🌱_🌻🌷'; | ||
return '🌵__🐍_🦂_🌵🌵__🐍_🏜_🦂'; | ||
}; | ||
|
||
const getFontColor = (temp) => { | ||
if (temp <= 49) return 'teal'; | ||
if (temp <= 59) return 'green'; | ||
if (temp <= 69) return 'yellow'; | ||
if (temp <= 79) return 'orange'; | ||
return 'red'; | ||
}; | ||
Comment on lines
+57
to
+70
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? |
||
|
||
increaseTempControl.addEventListener('click', () => { | ||
temperature += 1; | ||
updateTemperature(); | ||
}); | ||
|
||
decreaseTempControl.addEventListener('click', () => { | ||
temperature -= 1; | ||
updateTemperature(); | ||
}); | ||
updateTemperature(); | ||
|
||
//getRealTimeTemp button updates the temperature when we update the city | ||
getRealTimeTempButton.addEventListener('click', () => { | ||
const city = headerCityName.textContent; | ||
|
||
getCityLocation (city) | ||
.then(({ lat, lon }) => { | ||
return getCityTemperature(lat,lon); | ||
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 don't think you actually need to return this since you aren't chaining it to anything else. |
||
}) | ||
.catch((error) => { | ||
console.log('Error data:', error); | ||
tempValue.textContent = 'Unable to get temperature'; | ||
}) | ||
Comment on lines
+85
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. I would also suggest moving this into its own function(s), things could get tricky to debug (especially working with chained promises inside of callback functions. |
||
}); | ||
|
||
const getCityLocation = (city) => { | ||
return axios | ||
.get('https://ada-weather-report-proxy-server.onrender.com/location', {params:{'q': city}}) | ||
.then((locationResponse) => { | ||
console.log(locationResponse); | ||
const lat = locationResponse.data[0].lat; | ||
const lon = locationResponse.data[0].lon; | ||
console.log({ lat, lon }); | ||
return { lat, lon }; | ||
}) | ||
.catch((error) => { | ||
console.log('Error data:', error); | ||
throw error | ||
}); | ||
}; | ||
Comment on lines
+97
to
+111
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 this function! |
||
|
||
const getCityTemperature = (lat, lon) => { | ||
return axios | ||
.get('https://ada-weather-report-proxy-server.onrender.com/weather', {params:{"lat": lat, "lon": lon}}) | ||
.then((tempResponse) => { | ||
const kelvinTemp = tempResponse.data.main.temp; | ||
const fahrenheitTemp = Math.floor(((kelvinTemp - 273.15) * 9) / 5 + 32); | ||
tempValue.textContent = `${fahrenheitTemp}°F`; | ||
tempValue.style.color = getFontColor(fahrenheitTemp); | ||
landScape.textContent = getLandscape(fahrenheitTemp); | ||
Comment on lines
+119
to
+121
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. If we restructured our code to use the state object that was talked about inside of the reading we could actually move this logic into their respective functions. This would help with SRP and make our code easier to debug and maintain. Essentially, we would just replace the code here with something like |
||
}) | ||
.catch((error) => { | ||
console.log('Error data:', error); | ||
throw error | ||
}); | ||
}; | ||
|
||
//Update the city name when we input the city and headerCityName got update | ||
//When we click reset, it comes back to our default setting | ||
const updateCityName = (updateCity) => { | ||
headerCityName.textContent = updateCity; | ||
}; | ||
|
||
cityNameInput.addEventListener('input', (event) => { | ||
updateCityName(event.target.value); | ||
}); | ||
|
||
cityNameReset.addEventListener('click', () => { | ||
cityNameInput.value = ''; | ||
updateCityName('Seattle'); | ||
}); | ||
Comment on lines
+135
to
+142
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. These could also be moved into the |
||
|
||
|
||
|
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; | ||
background-color: #DFFFFF; | ||
transition: background-color 0.5s; | ||
} | ||
|
||
.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.
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.