Skip to content

Jacob's Weather Project #211

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

Open
jacobecox wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Jacob's Weather Project #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 45 additions & 0 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<!DOCTYPE html>
<html lang="en">
<head>
<link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css" integrity="sha384-ggOyR0iXCbMQv3Xipma34MD+dH/1fQ784/j6cY/iJTQUOhcWr7x9JvoRxT2MZw1T" crossorigin="anonymous">
<link rel="stylesheet" href="style.css">
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Weather App</title>
</head>
<body>

<div class="container">
<div>
<div>
<div class="page-header">
<h1 class="title">Weather</h1>
</div>

<div>
<form class="search-form container">
<input type="text" id="search-query" class="form" placeholder="Search your location">
</input>
<div>
<button type="button" class="btn btn-primary search">Search</button>
</form>
</div>
</div>



<hr/>

<div class="current-weather row">
<!-- Append current weather data here -->
</div>
<div class="weather-forecast row">
<!-- Append 5 day forecast here -->
</div>
</div>
</div>
</div>

<script src='main.js'></script>
</body>
</html>
220 changes: 220 additions & 0 deletions main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
// Set current date
const date = new Date();
const options = { weekday: 'long'};
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

options is too generic, dateOptions is a more specific name

const formattedDate = new Intl.DateTimeFormat('en-US', options).format(date);


const formattedDates = [];

for (let i = 0; i < 5; i++) {
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

next time wrap this in a function to give context of the purpose of this code

const nextDay = new Date(date);
nextDay.setDate(date.getDate() + i + 1);
const formattedDate = new Intl.DateTimeFormat('en-US', options).format(nextDay);
formattedDates.push(formattedDate);
}

// Get current weather

var currentWeather = [];

var weatherForecast = [];
Comment on lines +18 to +20
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

started using const, why not let?


document.querySelector('.search').addEventListener('click', function () {
var searchLocation = document.querySelector('#search-query').value;
var words = searchLocation.split(' ');
var city = words.join('/');
Comment on lines +23 to +25
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

var is not es6


document.querySelector('#search-query').value = '';
currentWeather = [];
weatherForecast = [];

fetchLocation(city);
fetchPlace(city);
});

var fetchLocation = function(city) {
const apiKey = '0e79198cac257d0139d13e84e8617759'
const url = `http://api.openweathermap.org/geo/1.0/direct?q=${city}&appid=` + apiKey;
fetch(url, {
method: 'GET',
dataType: 'json'
})
.then(data => data.json())
.then(data => fetchWeather(data));
};

var fetchWeather = function(data) {
const apiKey = '0e79198cac257d0139d13e84e8617759'
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

api key is the same, can be defined once at the top of the file or passed a param

const url = `https://api.openweathermap.org/data/2.5/weather?lat=${data[0].lat}&lon=${data[0].lon}&appid=` + apiKey + `&units=imperial`;
fetch(url, {
method: 'GET',
dataType: 'json'
})
.then(data => data.json())
.then(data => addWeather(data));
};

var addWeather = function (data) {
currentWeather.push({
temp: Math.round(data.main.temp),
name: data.name,
conditions: data.weather[0].main,
icon: `https://openweathermap.org/img/wn/${data.weather[0].icon}@4x.png`
})

renderWeather();
};

var renderWeather = function() {
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

not es6 Function declarations

document.querySelector('.current-weather').replaceChildren();

for (let i = 0; i < currentWeather.length; i++) {
const weather = currentWeather[i];

var template = `
<div class='col-md-4'>
<div class='weather'>
<h1>${weather.temp}º</h1>
<h2>${weather.name}</h2>
<h3>${weather.conditions}</h3>
</div>
</div>
<div class='col-md-4'>
<div class='icon'>
<img src=${weather.icon}>
</div>
</div>`;

document.querySelector('.current-weather').insertAdjacentHTML('beforeend', template);
}
};

// Get 5 day forecast

var fetchPlace = function(city) {
const url = 'http://api.openweathermap.org/geo/1.0/direct?q=';
const apiKey = '&appid=0e79198cac257d0139d13e84e8617759'
const fullUrl = url + city + apiKey;
fetch(fullUrl, {
method: 'GET',
dataType: 'json'
})
.then(data => data.json())
.then(data => fetchForecast(data));
};
Comment on lines +94 to +104
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

You are doing the same 3 times, you could have a generic function that takes api, url and callback for the data and reuse it.
Important DRY principle.

Suggested change
var fetchPlace = function(city) {
const url = 'http://api.openweathermap.org/geo/1.0/direct?q=';
const apiKey = '&appid=0e79198cac257d0139d13e84e8617759'
const fullUrl = url + city + apiKey;
fetch(fullUrl, {
method: 'GET',
dataType: 'json'
})
.then(data => data.json())
.then(data => fetchForecast(data));
};
const fetchPlace = (fullUrl, callback) => {
fetch(fullUrl, {
method: 'GET',
dataType: 'json'
})
.then(data => data.json())
.then(data => callBack(data));
};

Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

Way more readable!!!


var fetchForecast = function(data) {
const apiKey = '0e79198cac257d0139d13e84e8617759'
const url = `http://api.openweathermap.org/data/2.5/forecast?lat=${data[0].lat}&lon=${data[0].lon}&appid=${apiKey}&units=imperial`;

fetch(url, {
method: 'GET',
dataType: 'json'
})
.then(data => data.json())
.then(data => addForecast(data));
};

var addForecast = function (data) {
const dayOne = [];
const dayTwo = [];
const dayThree = [];
const dayFour = [];
const dayFive = [];

dayOne.push({
conditions: data.list[0].weather[0].main,
temp: Math.round(data.list[0].main.temp),
icon: `https://openweathermap.org/img/wn/${data.list[0].weather[0].icon}@2x.png`,
day: formattedDates[0]
})

dayTwo.push({
conditions: data.list[7].weather[0].main,
temp: Math.round(data.list[7].main.temp),
icon: `https://openweathermap.org/img/wn/${data.list[7].weather[0].icon}@2x.png`,
day: formattedDates[1]
})

dayThree.push({
conditions: data.list[15].weather[0].main,
temp: Math.round(data.list[15].main.temp),
icon: `https://openweathermap.org/img/wn/${data.list[15].weather[0].icon}@2x.png`,
day: formattedDates[2]
})

dayFour.push({
conditions: data.list[23].weather[0].main,
temp: Math.round(data.list[23].main.temp),
icon: `https://openweathermap.org/img/wn/${data.list[23].weather[0].icon}@2x.png`,
day: formattedDates[3]
})

dayFive.push({
conditions: data.list[31].weather[0].main,
temp: Math.round(data.list[31].main.temp),
icon: `https://openweathermap.org/img/wn/${data.list[31].weather[0].icon}@2x.png`,
day: formattedDates[4]
})
Comment on lines +125 to +158
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

DRY, could have used forEach day of the week.
Helper methods are your friends.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach



weatherForecast.push({
dayOne, dayTwo, dayThree, dayFour, dayFive
})

renderForecast();
};

var renderForecast = function() {
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

also this type of function could have been reused. You can create a more 'dumb' implementation that takes querySelector, template as params and makes the action

document.querySelector('.weather-forecast').replaceChildren();

for (let i = 0; i < weatherForecast.length; i++) {
const weather = weatherForecast[i];

var template = `
<div class='container-fluid'
<div class='row'>
<hr/>
</div>
</div>
<div class='header container-fluid'>
<h2><strong>5 Day Forecast</strong></h2>
</div>
<div id='forecast' class='container-fluid'>
<div class='row'>
<div class='day-one col-md-2 text-center'>
<h4>${weather.dayOne[0].conditions}</h4>
<h4>${weather.dayOne[0].temp}º</h4>
<img src= ${weather.dayOne[0].icon} >
<h4><strong>${weather.dayOne[0].day}</strong></h4>
</div>
<div class='day-two col-md-2 text-center'>
<h4>${weather.dayTwo[0].conditions}</h4>
<h4>${weather.dayTwo[0].temp}º</h4>
<img src= ${weather.dayTwo[0].icon} >
<h4><strong>${weather.dayTwo[0].day}</strong></h4>
</div>
<div class='day-three col-md-2 text-center'>
<h4>${weather.dayThree[0].conditions}</h4>
<h4>${weather.dayThree[0].temp}º</h4>
<img src= ${weather.dayThree[0].icon} >
<h4><strong>${weather.dayThree[0].day}</strong></h4>
</div>
<div class='day-four col-md-2 text-center'>
<h4>${weather.dayFour[0].conditions}</h4>
<h4>${weather.dayFour[0].temp}º</h4>
<img src= ${weather.dayFour[0].icon} >
<h4><strong>${weather.dayFour[0].day}</strong></h4>
</div>
<div class='day-five col-md-2 text-center'>
<h4>${weather.dayFive[0].conditions}</h4>
<h4>${weather.dayFive[0].temp}º</h4>
<img src= ${weather.dayFive[0].icon} >
<h4><strong>${weather.dayFive[0].day}</strong></h4>
</div>
</div>
</div>`;

document.querySelector('.weather-forecast').insertAdjacentHTML('beforeend', template);
}
};
73 changes: 73 additions & 0 deletions style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
body {
margin: 50px;
}

.title {
text-align: center;
font-size: 50px;
}

.search-form {
display: flex;
align-items: center;
}

.btn {
width: 100px;
}

hr {
border: 1px solid black;
}

.form {
flex: 1;
margin: 25px;
height: 30px;
}

.container {
width: 100%;
margin: 0 auto;
}
Comment on lines +29 to +32
Copy link

@ronyrosenberg ronyrosenberg Jun 15, 2024

Choose a reason for hiding this comment

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

the whole point of using bootstrap is to use to avoid creating the classes yourself.


.current-weather {
padding: 15px;
margin-bottom: 15px;
justify-content: center;
}

.day-one {
display: inline-block;
max-width: fit-content;
}

.day-two {
display: inline-block;
max-width: fit-content;
}

.day-three {
display: inline-block;
max-width: fit-content;
}

.day-four {
display: inline-block;
max-width: fit-content;
}

.day-five {
display: inline-block;
max-width: fit-content;
}

.header {
margin: 10px;
text-align: center;
}

.row {
justify-content: center;
text-align: center;
}