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.

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 45 additions & 3 deletions index.html
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>
157 changes: 157 additions & 0 deletions src/index.js
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 = () => {

Choose a reason for hiding this comment

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

👍

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 = () => {

Choose a reason for hiding this comment

The 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 = () => {

Choose a reason for hiding this comment

The 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

Choose a reason for hiding this comment

The 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 increaseTempControl or decreaseTempControl and depending on the id then you could ++ or --

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;

Choose a reason for hiding this comment

The 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;

Choose a reason for hiding this comment

The 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

Choose a reason for hiding this comment

The 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 changeColor()) or landscape emojis (for changeLandscape()).

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 landscape.textContent = state.niceLandscape;

const displayCityName = (event) => {
const cityInput = event.target.value;
state.cityNameOutput.textContent = cityInput;
};

const changeSky = () => {
sky.textContent = `${skySelect.options[skySelect.selectedIndex].text}`;

Choose a reason for hiding this comment

The 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) => {

Choose a reason for hiding this comment

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

Nitpick - there should be a whitespace before =

let latitude, longitude;

Choose a reason for hiding this comment

The 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:

state.latitude = response.data[0].lat;
state.longitude = response.data[0].lon;

axios.get('http://localhost:5000/location',

Choose a reason for hiding this comment

The 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}`,

Choose a reason for hiding this comment

The 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

Choose a reason for hiding this comment

The 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

Choose a reason for hiding this comment

The 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

Choose a reason for hiding this comment

The 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

Choose a reason for hiding this comment

The 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 registerEventHandlers on the event DOMContentLoaded, like this:

window.addEventListener("DOMContentLoaded", registerEventHandlers)

Then when your page loads registerEventHandlers will execute.

You could add loadControls() and setDefaultValues() inside registerEventHandlers()` so alll your 'start up' logic happens on page load.

164 changes: 164 additions & 0 deletions styles/index.css
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;
}