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.

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

Create Main.js #19

Closed
wants to merge 12 commits into from
Closed

Create Main.js #19

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f0709d8
Create Main.js
zkhing Feb 4, 2023
26040ad
moving Zaw's comments to app.js
SallyMcGrath Feb 4, 2023
0718549
deleting main component as we think we don't need this complexity
SallyMcGrath Feb 4, 2023
08fa292
Adding weather image
zkhing Feb 4, 2023
dac08f8
Fetching API
zkhing Feb 17, 2023
87d8e92
Button onclick
zkhing Feb 17, 2023
a18dd57
Setting temp
zkhing Feb 19, 2023
be36012
Fetch temp from API
zkhing Feb 21, 2023
85d523f
Matching up the weather icon images
zkhing Feb 24, 2023
ebb724f
Merge branch 'main' into main
LucyMac Feb 25, 2023
accfe59
Future weather and refactor code
zkhing Mar 1, 2023
544ee29
Merge branch 'main' of https://github.com/zkhing/RainyPlaytime into f…
zkhing Mar 1, 2023
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
1 change: 1 addition & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
21 changes: 20 additions & 1 deletion src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,35 @@
import "./App.scss";
import Header from "./components/Header/Header";
import Footer from "./components/Footer/Footer";
// import FakeWeatherData from "./data/FakeWeather.json";


//configs
const siteTitle = process.env.REACT_APP_SITE_TITLE ?? "CYF Weather";

function App() {

return (
<div className="App">
<Header title={siteTitle} />

<main className="c-site-main" tabIndex="0">
<div>
<img
className="weather-img"
src="https://cdn2.iconfinder.com/data/icons/weather-flat-14/64/weather02-512.png"
alt="Current Weather"
/>
</div>


{/* {FakeWeatherData.list.map((data) => (
<div key={data.dt}>Temp : {data && data.main.temp}</div>
))} */}

{/* <div>{location}</div> */}
{/* <div>{temp && temp.list[0].main.temp}</div> */}
</main>

<Footer />
</div>
);
Expand Down
12 changes: 12 additions & 0 deletions src/App.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,14 @@
@use "theme/utilities.scss";
@use "theme/global.scss";


.c-site-main{
background-color: skyblue;
}

.weather-img{
width: 200px;
height: 200px;
// max-width: 100%;
object-fit: cover;
}
47 changes: 41 additions & 6 deletions src/components/Header/Header.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,45 @@
import React from "react";
import './Header.scss'
import "./Header.scss";
import React, { useState, useEffect } from "react";

const Header = ({title}) =>
const Header = ({ title }) => {
const [data, setData] = useState([]);
const [city, setCity] = useState("");

function changeCity(){
setData(data);
}

useEffect(() => {
fetch(
`https://api.openweathermap.org/data/2.5/weather?q=liverpool&units=metric&appid=3b86046cce0de3be7cfa8369f4540b37`
)
.then((res) => res.json())
.then((data) => {
setData(data);
})
.catch((e) => console.log(e.message));
}, []);
Copy link

Choose a reason for hiding this comment

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

This is better but you are still saving the data in 2 different places (lines 9 and 18). You only need one.
I think what you need to do is:

  • onChange in the text input, save the user's input, just like you've done - don't change anything here it's perfect.
  • on search button click, trigger the API call to fetch the new data (for the new city we just saved). How will you do this? useEffect is good when you want to react to a change in value, but in this particular case, you want to react to a button click instead. I recommend not using useEffect here. How about just a simple getNewWeather function?
  • Replace the word 'liverpool' with ${city} so the API call changes.

If you do all this correctly it should update the temperature for each new city (it works for me 😊)

Copy link

Choose a reason for hiding this comment

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

(If you're wondering when to use useEffect, it would be perfect if you didn't have a 'search' button. For example if you just had the text input but no button, then you would want to make a new API call each time the user types a new letter. In this case, you would still save 'city' onChange but you would add city as a dependency in the useEffect array, and the API call inside the useEffect would get triggered each time the value of city changes.
This is very inefficient because you would make too many calls to the API, so in this case it's best to move the API call to be triggered by a button click instead.)

Copy link
Author

Choose a reason for hiding this comment

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

Thank you so much Lucy! It works now. I didn't realise that fetch API works without useEffect. :) Now I learned when to use useEffect.


return (
<header className="c-site-header">
<h1 className="c-site-header__title">{title}</h1>
</header>
<h1 className="c-site-header__title">{title}</h1>

<input
className="city"
type="text"
value={city}
onChange={(event) => setCity(event.target.value)}
/>

<button onClick={changeCity}>Search</button>

<div>
<h2>Temp:{data?.main?.temp?.toFixed()}°C</h2>
</div>

{/* {data.main.temp ? <h1>Temp:{data.main.temp}°C </h1> : null} */}
</header>
);
};

export default Header;
export default Header;
Loading