-
Notifications
You must be signed in to change notification settings - Fork 221
Weather App_eval3 #218
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
base: master
Are you sure you want to change the base?
Weather App_eval3 #218
Changes from all commits
5059d1c
e1980c1
fe4c8a9
17fa049
084a597
bab6b48
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 |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
|
|
||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Weather App Project</title> | ||
| <link href="https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/css/bootstrap.min.css" rel="stylesheet" | ||
| integrity="sha384-QWTKZyjpPEjISv5WaRU9OFeRpok6YctnYmDr5pNlyT2bRjXh0JMhjY6hW+ALEwIH" crossorigin="anonymous"> | ||
| <link href="style.css" rel="stylesheet"> | ||
| <script src="main.js" defer></script> | ||
| </head> | ||
|
|
||
| <body> | ||
| <h1>Weather Project</h1> | ||
| <hr> | ||
| <div class="input-group mb-3"> | ||
| <input type="text" class="form-control" placeholder="Input City" /> | ||
| <button type="button" class="btn btn-primary">Search</button> | ||
| </div> | ||
| <div id="weather-container"> | ||
| <div class="current-weather"> | ||
| <h2 id="city-name"></h2> | ||
| <img id="current-weather-icon" alt="Weather icon"> | ||
| <p id="current-temp"></p> | ||
| </div> | ||
| <div class="forecast"> | ||
| <h2>5-Day Forecast</h2> | ||
| <div id="forecast-days"> | ||
|
|
||
| </div> | ||
| </div> | ||
| </div> | ||
| </body> | ||
|
|
||
| </html> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| const apiKey = "ee02afd279ba852f5518dbe94195f595"; | ||
|
|
||
| const searchButton = document.querySelector(".btn-primary"); | ||
| const cityInput = document.querySelector('.form-control'); | ||
| const weatherContainer = document.getElementById('weather-container'); | ||
| const currentWeatherDiv = document.querySelector('.current-weather'); | ||
| const forecastDiv = document.querySelector('.forecast'); | ||
|
|
||
| currentWeatherDiv.style.display = 'none'; | ||
| forecastDiv.style.display = 'none'; | ||
|
|
||
| searchButton.addEventListener('click', () => { | ||
| const city = cityInput.value; | ||
| if (!city) { | ||
| alert('No city name given'); | ||
| return; | ||
| } | ||
| fetchCurrentWeather(city); | ||
| fetchForecast(city); | ||
| }); | ||
|
|
||
| function fetchCurrentWeather(city) { | ||
| const url = `https://api.openweathermap.org/data/2.5/weather?q=${city}&units=imperial&appid=${apiKey}`; | ||
| fetch(url) | ||
| .then((response) => response.json()) | ||
| .then(function (data) { | ||
| const cityNameElement = document.getElementById('city-name'); | ||
| const weatherIconElement = document.getElementById('current-weather-icon'); | ||
| const tempElement = document.getElementById('current-temp'); | ||
|
|
||
| cityNameElement.textContent = data.name; | ||
| weatherIconElement.src = `https://openweathermap.org/img/wn/${data.weather[0].icon}@2x.png`; | ||
| weatherIconElement.alt = data.weather[0].description; | ||
| tempElement.textContent = `${data.main.temp} °F - ${data.weather[0].description}`; | ||
|
|
||
| currentWeatherDiv.style.display = 'block'; | ||
| }) | ||
| .catch((error) => { | ||
| console.error('Error, didn\'t fetch weather', error); | ||
|
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. this gives no feedback to the user
Author
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. What would be the corrected code to give feedback to the user? 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. ideally would be an actual message that is rendered in the UI but for now alert could work as well |
||
| }); | ||
| } | ||
|
|
||
| function fetchForecast(city) { | ||
|
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. this function is too big. Need to be extracted into smaller chunks to help with readability and maintainability
Author
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. Thanks, Rony. How would this look as an example? 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. the function is getting data and adding it to the dom, you can create a get data function that returns a promise of that data and then split the for loop logic as well. |
||
| const url = `https://api.openweathermap.org/data/2.5/forecast?q=${city}&units=imperial&appid=${apiKey}`; | ||
| fetch(url) | ||
| .then((response) => response.json()) | ||
| .then(function (data) { | ||
| const forecastDaysElement = document.getElementById("forecast-days"); | ||
| forecastDaysElement.innerHTML = ''; // | ||
|
|
||
| for (let i = 0; i < data.list.length; i += 8) { // | ||
| const entry = data.list[i]; | ||
| const date = new Date(entry.dt * 1000); | ||
| const dayName = date.toLocaleDateString("en-US", { weekday: "long" }); | ||
|
|
||
| const forecastDayDiv = document.createElement('div'); | ||
| forecastDayDiv.className = 'forecast-day'; | ||
|
|
||
| const dayElement = document.createElement('h3'); | ||
| dayElement.textContent = dayName; | ||
|
|
||
| const weatherIcon = document.createElement('img'); | ||
| weatherIcon.src = `https://openweathermap.org/img/wn/${entry.weather[0].icon}@2x.png`; | ||
|
|
||
| const tempElement = document.createElement('p'); | ||
| tempElement.textContent = `${entry.main.temp} °F - ${entry.weather[0].description}`; | ||
|
|
||
| forecastDayDiv.appendChild(dayElement); | ||
| forecastDayDiv.appendChild(weatherIcon); | ||
| forecastDayDiv.appendChild(tempElement); | ||
| forecastDaysElement.appendChild(forecastDayDiv); | ||
| } | ||
|
|
||
| forecastDiv.style.display = 'block'; | ||
| }) | ||
| .catch((error) => { | ||
| console.error('Error, didn\'t fetch forecast:', error); | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| h1 { | ||
|
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. many styles here and layouts could have been bootstrap classes |
||
| text-align: center; | ||
| } | ||
|
|
||
| .form-control { | ||
| padding-left: 5; | ||
| margin-left: 502px; | ||
| } | ||
|
|
||
| .btn { | ||
| margin-right: 500px; | ||
| } | ||
|
|
||
| .current-weather { | ||
| text-align: center; | ||
| } | ||
|
|
||
| .current-weather img { | ||
| width: 100px; | ||
| height: 100px; | ||
| } | ||
|
|
||
| .current-weather h2 { | ||
| margin-bottom: 10px; | ||
|
|
||
| } | ||
|
|
||
| .current-weather p { | ||
| margin: 5px 0; | ||
| } | ||
|
|
||
| .forecast { | ||
| margin-top: 20px; | ||
| } | ||
|
|
||
| .forecast h2 { | ||
| text-align: center; | ||
| margin-bottom: 20px; | ||
| } | ||
|
|
||
| .forecast-day { | ||
| display: inline-block; | ||
| width: calc(20% - 16px); | ||
| margin: 0 3px; | ||
| text-align: center; | ||
| padding: 10px; | ||
| background-color: #ffffff; | ||
| border: 1px solid #dee2e6; | ||
| border-radius: 8px; | ||
| box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1); | ||
| } | ||
|
|
||
| .forecast-day img { | ||
| display: block; | ||
| margin: 0 auto 10px; | ||
| width: 80px; | ||
| height: 80px; | ||
| } | ||
|
|
||
| .forecast-day h3 { | ||
| margin-bottom: 8px; | ||
| } | ||
|
|
||
| .forecast-day p { | ||
| margin: 4px 0; | ||
| } | ||
|
|
||
|
|
||
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.
here the input needs to be trimmed
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.
ahh, yes. Thanks for this catch!