-
Notifications
You must be signed in to change notification settings - Fork 177
Added a weather app to fetch and display weather by city name #328
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
Conversation
@ArshadShaik07 is attempting to deploy a commit to the Suman Kunwar's projects Team on Vercel. A member of the Team first needs to authorize it. |
@ArshadShaik07 The solution looks great! Would it be possible to place it inside a folder (weather app)? That way, we can easily add more examples later. |
yeah sure. |
What about now? |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Pull Request Overview
This PR adds a weather application that allows users to fetch and display weather information by entering a city name. The app uses the OpenWeatherMap API to retrieve weather data and presents it in a responsive web interface.
- Implements weather data fetching from OpenWeatherMap API
- Creates responsive UI with input field, search button, and weather display sections
- Handles user interactions including button clicks and Enter key press
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
File | Description |
---|---|
examples/Weather-app/index.html | HTML structure with input field, search button, and weather display containers |
examples/Weather-app/script.js | JavaScript functionality for API calls, DOM manipulation, and event handling |
examples/Weather-app/styles.css | CSS styling for responsive design and modern UI appearance |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
examples/Weather-app/script.js
Outdated
const btn = document.querySelector(".srch-btn"); | ||
const cityOp = document.querySelector(".city-name-output"); | ||
|
||
const apiKey = "6244ab888f079565d5ce1deabddc3f77"; |
Copilot
AI
Oct 4, 2025
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.
API key is hardcoded and exposed in the client-side code. This key should be moved to environment variables or a secure backend service to prevent unauthorized usage.
Copilot uses AI. Check for mistakes.
examples/Weather-app/script.js
Outdated
); | ||
console.log(res); | ||
if (!res.ok) { | ||
throw Error("Error occured.Could not find the city "); |
Copilot
AI
Oct 4, 2025
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.
Corrected spelling of 'occured' to 'occurred'.
throw Error("Error occured.Could not find the city "); | |
throw Error("Error occurred. Could not find the city "); |
Copilot uses AI. Check for mistakes.
windspeed.innerHTML = s; | ||
temp.innerHTML = t + "<sup>o</sup>C" + `<p>${desc}</p>`; | ||
humidity.innerHTML = h; | ||
feels_like.innerHTML = fl + "<sup>o</sup>C"; |
Copilot
AI
Oct 4, 2025
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.
Using innerHTML with dynamic content can lead to XSS vulnerabilities. Consider using textContent or properly sanitizing the content before insertion.
feels_like.innerHTML = fl + "<sup>o</sup>C"; | |
feels_like.textContent = fl; | |
const sup = document.createElement("sup"); | |
sup.textContent = "o"; | |
feels_like.appendChild(sup); | |
feels_like.appendChild(document.createTextNode("C")); |
Copilot uses AI. Check for mistakes.
examples/Weather-app/index.html
Outdated
<head> | ||
<meta charset="UTF-8"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<title>Document</title> |
Copilot
AI
Oct 4, 2025
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 page title should be more descriptive than 'Document'. Consider changing it to 'Weather App' to match the application's purpose.
<title>Document</title> | |
<title>Weather App</title> |
Copilot uses AI. Check for mistakes.
window.addEventListener("load", () => { | ||
getWeatherInfo("kurnool"); | ||
}); |
Copilot
AI
Oct 4, 2025
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.
[nitpick] The hardcoded default city 'kurnool' should be configurable or removed. Consider using a more universally recognizable default city or removing the default load behavior.
window.addEventListener("load", () => { | |
getWeatherInfo("kurnool"); | |
}); |
Copilot uses AI. Check for mistakes.
Thank you . |
@ArshadShaik07 When you have time, can you look into above suggestions? |
Sure . |
I have made the modifications according to your suggestions @sumn2u .Kindly check it once |
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.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
windspeed.innerHTML = s; | ||
temp.innerHTML = t + "<sup>o</sup>C" + `<p>${desc}</p>`; | ||
humidity.innerHTML = h; | ||
feels_like.innerHTML = fl + "<sup>o</sup>C"; | ||
cityOp.innerHTML = n; | ||
// Use textContent to prevent XSS vulnerabilities | ||
windspeed.textContent = s; | ||
humidity.textContent = h; | ||
cityOp.textContent = n; |
Copilot
AI
Oct 5, 2025
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.
Lines 52-56 set innerHTML values that are immediately overwritten by textContent assignments in lines 58-60. The initial innerHTML assignments are redundant and should be removed.
Copilot uses AI. Check for mistakes.
if (res.status === 404) { | ||
throw Error("City not found"); | ||
} else if (res.status === 401) { | ||
throw Error("Access denied.Check your api key "); |
Copilot
AI
Oct 5, 2025
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.
Missing space after period in error message. Should be 'Access denied. Check your api key'.
throw Error("Access denied.Check your api key "); | |
throw Error("Access denied. Check your api key"); |
Copilot uses AI. Check for mistakes.
// IMPORTANT: Replace "YOUR_API_KEY" with your actual OpenWeatherMap API key. | ||
const apiKey = "YOUR_API_KEY"; | ||
async function getWeatherInfo(city) { | ||
try { | ||
console.log(city); |
Copilot
AI
Oct 5, 2025
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.
API key is hardcoded as a placeholder string. This should be moved to environment variables or a configuration file to prevent accidental exposure in version control.
// IMPORTANT: Replace "YOUR_API_KEY" with your actual OpenWeatherMap API key. | |
const apiKey = "YOUR_API_KEY"; | |
async function getWeatherInfo(city) { | |
try { | |
console.log(city); | |
// IMPORTANT: Set your OpenWeatherMap API key as a global variable before this script runs. | |
// Example: <script>window.OPENWEATHERMAP_API_KEY = "your_actual_api_key";</script> | |
const apiKey = window.OPENWEATHERMAP_API_KEY; | |
async function getWeatherInfo(city) { | |
console.log(city); | |
if (!apiKey) { | |
throw Error("API key is not set. Please set window.OPENWEATHERMAP_API_KEY before loading this script."); | |
} |
Copilot uses AI. Check for mistakes.
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.
LGTM
Fixes #327 |
This PR adds a simple weather app that allows users to check the weather based on a city name input.
Features:
This project demonstrates API fetching, DOM manipulation, and basic JavaScript usage.