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

Ashley_Benson_Seaturtles #19

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+211,581 −1
Open

Ashley_Benson_Seaturtles #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8cd5ac0
Wave 2
Jun 10, 2022
e9a51b6
Done
Jun 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
57 changes: 56 additions & 1 deletion index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,67 @@
<!DOCTYPE html>
<html lang="en">

Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't a Python project, so we didn't need to create a venv. If this were a Python project, we'd want to make sure that vevn was listed in the .gitignore, since the venv should not get checked in.

<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="stylesheet" href="styles/index.css" />
</head>

<body>


<header class="header_section">
<h1>Ashley's Weather Report</h1>
<span class="subheader">
<span id="headerCityName" class="header_city"></span>
</span>
Comment on lines +15 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

Good choice to not use h2 as the subheading. h2 is a heading in its own right, and isn't intended to just be the subtitle to an h1 heading.

</header>

<div class="input_container">
<i class="fa-thin fa-magnifying-glass"></i>
<input class="input_field" type="text" placeholder="Search City" name="city search" />
</div>
Comment on lines +21 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like this field is used for the realtime city. The realtime lookup uses #cityNameInput so this input could be removed.


<div class="main_area">
<button id="realTime" class="realTimeButton">Real Time</button>
<section class="temperature_area">
<h2 id="temp_caption">Temperature</h2>
<div class="temp_txt"></div>
<div class="temp_wheel"></div>
<span id="increase_temp">⬆️</span>
<span id="decrease_temp">⬇️</span>
<div>
<span id="temp_value"></span>
</div>

</section>
<section class="skyArea">
<h2 id="skyCaption">Sky</h2>
<select id="skySelect">
<option value="none" selected disabled hidden>select sky</option>
<option value="sun">Sunshine</option>
<option value="overcast">Overcast</option>
<option value="rain">Rain Storm</option>
<option value="snow">Snow Storm</option>
</select>
</section>
<section class="city_section">
<h2 id="cityTitle">City Name</h2>
<input type="text" id="cityNameInput" value="Seattle 🌧" />
<button id="cityNameReset" class="resetButton">Reset</button>
</section>
<section class="gardenArea">
<h2 id="gardenCaption">Weather Garden</h2>
<div id="gardenStuff" class="garden">
<div id="sky"></div>
<div id="landscape"></div>
</div>
</section>
</div>
<script src="src/index.js"></script>
<script src="./node_modules/axios/dist/axios.min.js"></script>

</body>

</html>
116 changes: 116 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
"use strict";
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to make git commits as you work. The README asked us to make at least 5 commits (1 per wave is a good start).






let temp_value = 72
let sky = ""

const updateCity = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

const getCityName = document.getElementById("cityNameInput").value;
const headerCityContainer = document.getElementById("headerCityName");
headerCityContainer.textContent = getCityName;
};
// reset to default city name
const resetCity = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Nice reuse of your update function.

const cityContainer = document.getElementById("cityNameInput");
cityContainer.value = "Seattle 🌧";
// reflect that reset on the header
updateCity();
};
const updateTemp = function(tempValue) {
const tempValueContainer = document.getElementById("temp_value");
const landscapeContainer = document.getElementById("landscape");
temp_value = tempValue
// console.log(tempValueContainer)
tempValueContainer.textContent = tempValue;
if (tempValue >= 80) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Notice how repetitive the temp check is combined with the element data being set. Consider moving information about the temperature bounds and the color and landscape info into a list of objects that we could iterate through to find the right record with the values for us to use.

tempValueContainer.style.color = "red";
landscapeContainer.textContent = "🌵__🐍_🦂_🌵🌵__🐍_🏜_🦂";
} else if (tempValue >= 70) {
tempValueContainer.style.color = "orange";
landscapeContainer.textContent = "🌸🌿🌼__🌷🌻🌿_☘️🌱_🌻🌷";
} else if (tempValue >= 60) {
tempValueContainer.style.color = "gold";
landscapeContainer.textContent = "🌾🌾_🍃_🪨__🛤_🌾🌾🌾_🍃";
} else if (tempValue >= 50) {
tempValueContainer.style.color = "green";
landscapeContainer.textContent = "🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲";
} else {
tempValueContainer.style.color = "teal";
landscapeContainer.textContent = "🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲";
}
};

const skyChanger = () => {
const skyContainer = document.getElementById("sky");

if (document.getElementById('skySelect').value === 'sun') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Like for the temp check each of these clauses is very repetitive. We look for a matching value to locate information about what values to set in the UI. Consider using object (key) lookups to make this more data-driven. If the values are stored elsewhere (maybe loaded from an api) then the code here would essentially reduce down to a key lookup in an object!

document.querySelector('.skyArea').style.background = "#FCE762";
skyContainer.textContent = "☁️☁️☁️☁️☁️☀️☁️☁️☁️☁️";
}
if (document.getElementById('skySelect').value === 'overcast') {
document.querySelector('.skyArea').style.background = "#607B7D";
skyContainer.textContent = "☁️☁️ ☁️ ☁️☁️ ☁️ 🌤 ☁️ ☁️☁️"
}
if (document.getElementById('skySelect').value === 'rain') {
document.querySelector('.skyArea').style.background = "#5100B3";
skyContainer.textContent = "🌧🌈⛈🌧🌧💧⛈🌧🌦🌧💧🌧🌧"
}
if (document.getElementById('skySelect').value === 'snow') {
document.querySelector('.skyArea').style.background = "#76E5FC";
skyContainer.textContent = "🌨❄️🌨🌨❄️❄️🌨❄️🌨❄️❄️🌨🌨"
}
}

const getRealTimeTemp = () => {
const cityName = document.getElementById("cityNameInput").value;
axios.get("http://127.0.0.1:5000/location", { params: { q: cityName } }).then(
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider storing the unchanging part of the URL (the base URL) in a variable elsewhere, then use string interpolation to build the full path here. This would make it easier to change where all of the endpoints in the app are going by changing a single variable.

(response) => {
const lat1 = response.data[0].lat
const lon1 = response.data[0].lon
axios.get("http://127.0.0.1:5000/weather", { params: { lat: lat1, lon: lon1 } }).then(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nesting another axios call in the response handler works out OK in this situation since the two calls we're making are pretty small and related, so it makes sense to have them grouped together under the getRealTimeTemp function. But it can also be useful to make these two calls into individual helpers, and then combining them in another function. Something like (very loosely):

const getLatLon = (...) => { return axios.get(...) };
const getTemp = (...) => { return axios.get(...) };
const getTempForCity = (...) => {
  return getLatLon(...)
    .then(response => getTemp(response.data.latLonData))
    .then(response => publishTempData(response.data.tempData))
    .catch(err => console.log(err))
};

(response) => {
const realTemp = response.data.current.temp
const farTemp = Math.round((realTemp - 273.15) * 1.8000 + 32.00)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider moving this conversion code into a helper function.

updateTemp(farTemp);

}

)
}

)
};

const registerEventHandlers = function() {
const skyHelper = document.getElementById('skySelect');
skyHelper.addEventListener('change', skyChanger);
updateTemp(temp_value);

const incButton = document.querySelector("#increase_temp");
incButton.addEventListener("click", function() {
temp_value += 1;
updateTemp(temp_value);

});
Comment on lines +93 to +97
Copy link
Contributor

Choose a reason for hiding this comment

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

Using function(){} here won't make a real difference compared to an arrow function, but generally prefer arrow functions as they have less "surprising" behavior.

Nice anonymous handler that updates the temp, then applies the changes to the page. Rather than modifying the temp and passing that in, consider passing in a delta value and having the temp be modified inside updateTemp. Then this could look like

    incButton.addEventListener("click", () => {
        updateTemp(1);
    });

const realTimeButton = document.querySelector("#realTime")
realTimeButton.addEventListener("click", getRealTimeTemp)

const decButton = document.querySelector("#decrease_temp");
decButton.addEventListener("click", function() {
temp_value -= 1;
updateTemp(temp_value);
})
const cityContainer = document.querySelector("#cityNameReset");
cityContainer.addEventListener("click", resetCity)

const cityNameInput = document.querySelector("#cityNameInput")
cityNameInput.addEventListener("input", updateCity)
}



// registerEventHandlers()
document.addEventListener("DOMContentLoaded", registerEventHandlers);
130 changes: 130 additions & 0 deletions styles/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
body {
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
background-color: #EDD4FD;
}

.input_field {
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little surprised that you needed to set the font-family in so many classes. font-family is an inherited property so in theory shouldn't need to be set so frequently. If it's not "taking", some alternatives would be to make a class that sets the app font, and apply that class to any elements that need to use the app font (in addition to any other classes on the element). Alternatively, you could look into CSS vars, which would allow you to define the list of fonts once, then refer to that list in all the rules that need it.

}

.input_container {
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
border-radius: 5px;
}

h1 {
display: flex;
align-items: left;
justify-content: left;
}

.subheader {
display: flex;
font-size: 1rem;
color: #D193FA;
margin-bottom: 30px;
justify-content: center;
}

.main_area {
display: flex;
flex-direction: row;
justify-content: space-evenly;
border: thin;
}

.temperature_area {
background-color: rgba(81, 0, 179, 0.415);
display: flex;
align-items: center;
padding: 0px 10px 5px 10px;
border-radius: 5px;
}

#temp_value {
font-size: 33px;
margin-right: 1rem;
justify-content: center;
}

.temp_wheel {
display: flex;
flex-direction: column;
align-items: center;
margin-right: 3rem;
font-size: 18px;
}

.temp_txt {
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
font-size: 28px;
}

.skyArea {
background-color: rgba(81, 0, 179, 0.415);
padding: 0px 60px 0px 60px;
border-radius: 5px;
flex-direction: column;
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
}

#skySelect {
align-content: center;
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
}

.city_section {
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
background-color: rgba(81, 0, 179, 0.415);
;
padding: 0px 20px 0px 20px;
border-radius: 5px;
flex-direction: column;
}

#cityNameInput,
#cityNameReset {
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
}

#temperatureCaption,
#skyCaption,
#cityCaption {
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
margin: 10px;
text-align: center;
font-size: 24px;
}

.gardenArea {
background-color: rgba(81, 0, 179, 0.415);
;
/* display: flex; */
flex-direction: column;
justify-content: flex-end;
text-align: center;
padding: 0px 20px 0px 20px;
margin-top: 40px;
margin-left: 7.5%;
margin-right: 7.5%;
border-radius: 5px;
}

#gardenCaption {
margin-top: 15px;
text-align: center;
font-size: 24px;
}

.garden {
min-height: 200px;
display: flex;
flex-direction: column;
padding-bottom: 20px;
justify-content: space-between;
}

#sky,
#landscape {
font-size: 30px;
}
Loading