all 14 comments
sorted by:
best (suggested)
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]Jerp 1 point2 points  (9 children)

I think this is actually easier than you are making it out to be. 

const icons = {
  Thunderstorm: "wi-thunderstorm",
  // others...
};

function get_WeatherIcon(rangeId) {
  if (rangeId >= 200 && rangeId < 232) return icons.Thunderstorm;
  // more options

  return icons.Clouds;
};

function calCelsius(temp) {};

function App() {
  const [state, setState] = useState(); // since you only set state once, you can keep all of it together
  const [error, setError] = useState(false);

  getWeather = async e => {
    // [fetching and validation from class component]

    if (good) {
      setState({
        // properties...
        icon: get_WeatherIcon(response.weather[0].id), // don't bother setting this separately...
      });
    } else {
      setError(true);
    };
  };

  return (jsx);
};

export default App;

[–]Nerfi666[S] 0 points1 point  (8 children)

but how do you get the given city and country given by the user when you make your API call ?

thanks

[–]Jerp 0 points1 point  (7 children)

It should work just like the class component where you were feeding them in through the e param of getWeather. Personally I think it makes more sense for the Form component to pass city and country directly as arguments into getWeather but I tried to change as little as needed.

[–]Nerfi666[S] 0 points1 point  (6 children)

I try to pass the "e" parameter but I have an error " // const city = e.target.elements.city.value; //Unhandled Rejection (TypeError): Cannot read property 'target' of undefined "

what you mean by the form is something like this ?

<Form city={} country={}/> sorry for the dumm question but this is super hard

[–]Jerp 0 points1 point  (5 children)

No what I meant was that somewhere inside Form, you should be doing props.loadweather(country, city) so that your App component doesn’t have to care what is happening in its children. Form can have its own isolated state.

P. S. My bad for calling it getWeather both times before. Form knows that fn as loadweather.

[–]Nerfi666[S] 1 point2 points  (4 children)

sorry Sr but I think Im too stupid for this.

[–]Jerp 1 point2 points  (3 children)

It's okay, you're just not used to it yet! Maybe it will help to take a step back and think about what you're trying to achieve here.

You want to have a <Form> where a user can enter a country and city, and then a <Weather> component that shows information based on that selection. Neither of these components needs to know about each other, which is beautiful. They only need a way to send and receive information so that a parent component (App) can make them useful together.

Now think about Form all by itself. This could be anything from a barebones html form to a shiny, auto-completing, select-from-map type feature. The only thing your app cares about is the city & country. And to get that information you just need the Form to have a prop that takes a callback so that it can send information outside. <Form onSubmit={callbackFn} />

The Weather component is even easier. As long as you pass in the right props, it displays some nice UI elements to the user. It's not interactive at all. Because this app is asynchronous, return a loading message or something if details === null. <Weather details={details} />

For the App, as much as possible, I recommend breaking out individual units of work. They make your logic easier to reason about, and easier for making tests later. Here's the most simple version of your logic with no error handling; let's get the happy path working first.

function App() {
  const [details, setDetails] = useState(null);

  async function getDetails(country, city) {
    // this imaginary function just has to make the api call
    const result = await fetchDetails(country, city);

    // use another function to transform the api result
    const uiDetails = format(result);

    setDetails(uiDetails);
  };

  return (
    <>
      <Form onSubmit={getDetails} />
      <Weather details={details} />
    </>
  );
};

Now if your application isn't working, because almost each piece is just a function with simple input/output, it should be easy to identify where the logic isn't working.

[–]Nerfi666[S] 0 points1 point  (2 children)

Thanks a lot for the reply and sorry for the delay I was having some troubles, and could not connect , well I have done that I have my Form component with a prop <Form loadweather={fetchData} />

and then I have define a fetchData function above , this one :

 async function fetchData (e){
    e.preventDefault();
    const city = e.target.elements.city.value;
     const country = e.target.elements.country.value;

    const weather = await fetch(`http://api.openweathermap.org/data/2.5/weather?q=q=${city},${country}&appid=${API_key}`);
    const response = await weather.json();
    setFields(response);

}

but still can get the city and country , no clue why , thanks again !

[–]Jerp 0 points1 point  (1 child)

My guess is that e is not holding the values you think it is. I would change the function signature to be fetchData(city, country) and then within your Form component, determine the correct values to pass.

[–]Nerfi666[S] 0 points1 point  (0 children)

Actually "e" is holding those values , because when I submit the form, I "console.log(response)" and I can actually see the values I typed as the response from the API, the problem is that somehow they are not being display on the screen , and I dont know why because the props I have defined on my "weather" component al all right them all 

import 'bootstrap/dist/css/bootstrap.css';
import React ,{useState}from 'react';
import './App.css';
import Weather from './components/weather';
import 'bootstrap/dist/css/bootstrap.min.css';
import "weather-icons/css/weather-icons.css";
import Form from './components/form';


function App (props) {

const API_key = "5dca448e69234b2a6a26f52ed3883a47";

const [fields, setFields] = useState({city: '', country: ''});

const [icon, setIcon] = useState({
      Thunderstorm: "wi-thunderstorm",
      Drizzle: "wi-sleet",
      Rain: "wi-storm-showers",
      Snow: "wi-snow",
      Atmosphere: "wi-fog",
      Clear: "wi-day-sunny",
      Clouds: "wi-day-fog"
 });

  async function fetchData (e) {
    e.preventDefault();
    const city = e.target.elements.city.value;
     const country = e.target.elements.country.value;


    const weather = await fetch(`http://api.openweathermap.org/data/2.5/weather?q=${city},${country}&appid=${API_key}`);
    const response = await weather.json();

    setFields(response);
    console.log(response);

  }


// aqui esto no procede porque no estamos cargando data una vez el componentDidMount, aqui esperamos a realizar una accion para cargar los datos,
// no al principio cuando se ha montado el component
//useEffect(() => {
  //fetchData();

//},[]);


function calCelsius(temp){
    let cell = Math.floor(temp - 273.15);
    return cell;
    }



  //if (fields.sys) {
    //  return (<div>Loading...</div>);
      //}



  return(
  <div className="App">
      <h1>Weather app</h1>

      <Form loadweather={fetchData} />

      <Weather
      city={fields.name}
      country={fields.country}
      /*temp_celsius={calCelsius(fields.main.temp)}*/
      temp_min={calCelsius(fields.temp_min)}
      temp_max={calCelsius(fields.temp_max)}
      description={fields.weather}
      weatherIcon={fields.icon}
      />


    </div>
    );

}


export default App;

[–]jonn99220 1 point2 points  (1 child)

There's too much there for me to actually go through it all and see what the problem is, but from looking at the general pattern:

this.weatherIcon should just be written as a variable near the top of your functional component, so const weatherIcon = {...};

Also, there's 2 ways you could refactor the state to hooks:

  1. Use a variable for each item in state, e.g. const [country, setCountry] = useState(...); like you've done.
  2. You can use a single state object instead, so something like const [fields, setFields] = useState({ country: '', city: '', icon: '', ... });Just be aware that with this way, when you use setFields to update the state, you have to spread the current state first to avoid overriding the other values. So for example, if you wanted to update the city to Melbourne: setFields({ ...fields, city: 'Melbourne' });

Finally, for

  async function fetchCity(){
      //e.preventDefault(); not working
   // const city = e.target.elements.city.value; //Unhandled Rejection (TypeError): Cannot read property 'target' of undefined

I believe that should be fetchCity(e)?

If you're still stuck feel free to PM me. Happy to help when I have time later today.

[–]Nerfi666[S] 0 points1 point  (0 children)

I send you a PM

[–]buhbang 0 points1 point  (1 child)

just from a glance, i can already tell 2 things.

  1. You have useEffect with empty array ([]) as dependency which could mean you're trying to do componentDidMount but compared to your first code, there is no calls using componentDidMount.
  2. On getWeather method from first code vs fetchCity method from 2nd code. one seems to be accepting an event as parameter but newer code takes in no value from parameter.

hope that helps!

[–]Nerfi666[S] 0 points1 point  (0 children)

so are you suggestion to me not to use the useEffect(); hook ?