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

Test package #2

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
12 changes: 12 additions & 0 deletions .github/workflows/console.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: My Workflow

on:
pull_request:
types: [opened, reopened, synchronize]

jobs:
check_logs:
runs-on: ubuntu-latest
steps:
- name: Check Logs
uses: SyedSibtainRazvi/check_logs@main
52 changes: 30 additions & 22 deletions src/App.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,35 @@
import React, { useState } from 'react'
import axios from 'axios'
import React, { useState } from "react";
import axios from "axios";

function App() {
const [data, setData] = useState({})
const [location, setLocation] = useState('')
const [data, setData] = useState({});
const [location, setLocation] = useState("");

const url = `https://api.openweathermap.org/data/2.5/weather?q=${location}&units=metric&appid=895284fb2d2c50a520ea537456963d9c`
const url = `https://api.openweathermap.org/data/2.5/weather?q=${location}&units=metric&appid=895284fb2d2c50a520ea537456963d9c`;

const searchLocation = (event) => {
if (event.key === 'Enter') {
if (event.key === "Enter") {
axios.get(url).then((response) => {
setData(response.data)
console.log(response.data)
})
setLocation('')
setData(response.data);
console.log(response.data);
});
setLocation("");
}
Copy link

Choose a reason for hiding this comment

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

Consider handling potential errors from the axios.get call with a .catch block to improve robustness and error handling.

axios.get(url).then((response) => {
  setData(response.data);
  console.log(response.data);
}).catch((error) => {
  console.error("Error fetching data:", error);
});
setLocation("");

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
setData(response.data);
console.log(response.data);
});
setLocation("");
setData(response.data);
console.log(response.data);
}).catch((error) => {
console.error("Error fetching data:", error);
});
setLocation("");

}
};
console.log(data);

return (
<div className="app">
<div className="search">
<p>WeatherU</p>
<input
value={location}
onChange={event => setLocation(event.target.value)}
onChange={(event) => setLocation(event.target.value)}
onKeyPress={searchLocation}
placeholder='Enter Location'
type="text" />
</div>
placeholder="Enter Location"
type="text"
/>
</div>
Comment on lines +25 to +29
Copy link

Choose a reason for hiding this comment

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

The JSX syntax for the input element is clear and readable. However, consider adding aria-label for accessibility improvements.

<input
  value={location}
  onChange={(event) => setLocation(event.target.value)}
  onKeyPress={searchLocation}
  placeholder="Enter Location"
  type="text"
+ aria-label="Location"
/>

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
onChange={(event) => setLocation(event.target.value)}
onKeyPress={searchLocation}
placeholder='Enter Location'
type="text" />
</div>
placeholder="Enter Location"
type="text"
/>
onChange={(event) => setLocation(event.target.value)}
onKeyPress={searchLocation}
placeholder="Enter Location"
type="text"
aria-label="Location"
/>

<div className="container">
<div className="top">
<div className="location">
Expand All @@ -37,29 +39,35 @@ function App() {
{data.main ? <h1>{data.main.temp.toFixed()}°C</h1> : null}
</div>
<div className="description">
{data.weather ? <p className='bold'>{data.weather[0].main}</p> : null}
{data.weather ? (
<p className="bold">{data.weather[0].main}</p>
) : null}
</div>
</div>

{data.name !== undefined &&
{data.name !== undefined && (
<div className="bottom">
<div className="feels">
{data.main ? <p className='bold'>{data.main.feels_like.toFixed()}°C</p> : null}
{data.main ? (
<p className="bold">{data.main.feels_like.toFixed()}°C</p>
) : null}
<p>Minimum</p>
</div>
<div className="humidity">
{data.main ? <p className='bold'>{data.main.humidity}%</p> : null}
{data.main ? <p className="bold">{data.main.humidity}%</p> : null}
<p>Humidity</p>
</div>
<div className="wind">
{data.wind ? <p className='bold'>{data.wind.speed.toFixed()} MPH</p> : null}
{data.wind ? (
<p className="bold">{data.wind.speed.toFixed()} MPH</p>
) : null}
<p>Wind Speed</p>
</div>
</div>
}
)}
</div>
Comment on lines +46 to +65
Copy link

Choose a reason for hiding this comment

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

The conditional rendering block for the "bottom" div is well-implemented. However, consider extracting this into a separate component for better readability and maintainability.

+ const WeatherDetails = ({ data }) => (
+   {data.name !== undefined && (
+     <div className="bottom">
+       <div className="feels">
+         {data.main ? (
+           <p className="bold">{data.main.feels_like.toFixed()}°C</p>
+         ) : null}
+         <p>Minimum</p>
+       </div>
+       <div className="humidity">
+         {data.main ? <p className="bold">{data.main.humidity}%</p> : null}
+         <p>Humidity</p>
+       </div>
+       <div className="wind">
+         {data.wind ? (
+           <p className="bold">{data.wind.speed.toFixed()} MPH</p>
+         ) : null}
+         <p>Wind Speed</p>
+       </div>
+     </div>
+   )}
+ );

- {data.name !== undefined && (
-   <div className="bottom">
-     ...
-   </div>
- )}
+ <WeatherDetails data={data} />

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
{data.name !== undefined && (
<div className="bottom">
<div className="feels">
{data.main ? <p className='bold'>{data.main.feels_like.toFixed()}°C</p> : null}
{data.main ? (
<p className="bold">{data.main.feels_like.toFixed()}°C</p>
) : null}
<p>Minimum</p>
</div>
<div className="humidity">
{data.main ? <p className='bold'>{data.main.humidity}%</p> : null}
{data.main ? <p className="bold">{data.main.humidity}%</p> : null}
<p>Humidity</p>
</div>
<div className="wind">
{data.wind ? <p className='bold'>{data.wind.speed.toFixed()} MPH</p> : null}
{data.wind ? (
<p className="bold">{data.wind.speed.toFixed()} MPH</p>
) : null}
<p>Wind Speed</p>
</div>
</div>
}
)}
const WeatherDetails = ({ data }) => (
{data.name !== undefined && (
<div className="bottom">
<div className="feels">
{data.main ? (
<p className="bold">{data.main.feels_like.toFixed()}°C</p>
) : null}
<p>Minimum</p>
</div>
<div className="humidity">
{data.main ? <p className="bold">{data.main.humidity}%</p> : null}
<p>Humidity</p>
</div>
<div className="wind">
{data.wind ? (
<p className="bold">{data.wind.speed.toFixed()} MPH</p>
) : null}
<p>Wind Speed</p>
</div>
</div>
)}
);
<WeatherDetails data={data} />

</div>
);
}

export default App;
export default App;
Loading