CODEWITHSUNDEEP
reactjsreact-hooks
GoOsT
GoOsT

Reputation: 111

React hooks and fetch() advice

I have a component that fetches a random person, saves it to a state variable array and after mapping through that array, it returns certain elements. My goal is to render a "social card" with an avatar and personal infos, I want to use a material UI CARD component for this. Also I'd like to get any feedback on my logic here, whether this is a good way to achieve what I have achieved. The setTimeout() is only there so I can render some loading animation. Also when I tried to return a full card component inside the array map, I could not render {item.something.something}

export default function SocialCardsfunction() {
  const [items, setItems] = useState([]);
  const [loading, setLoading] = useState(true);

  const classes = useStyles();

  const nextPerson = () => {
    fetch("https://randomuser.me/api")
      .then((response) => response.json())
      .then((response) => {
        setItems(response.results);
      });
  };
  
  useEffect(() => {
    fetch("https://randomuser.me/api")
      .then((response) => response.json())
      .then((response) => {
        setTimeout(() => {
          setItems(response.results);
          setLoading(false);
        }, 1000);
      });
  }, []);

  if (loading) {
    return <div>Loading ...</div>;
  } else {
    return (
      <div>
        {items.map((item, i) => {
          return (
            <div>
              <h2>{item.name.first}</h2>
              <img src={item.picture.large} alt='picture' key={i} />
              <button onClick={() => nextPerson()}>click me</button>
            </div>
          );
        })}
      </div>
    );
  }
}

const useStyles = makeStyles({
  root: {
    minWidth: 275,
  },
  bullet: {
    display: "inline-block",
    margin: "0 2px",
    transform: "scale(0.8)",
  },
  title: {
    fontSize: 14,
  },
  pos: {
    marginBottom: 12,
  },
});

Upvotes: 0

Views: 139

Answers (2)

Mantas Astra
Mantas Astra

Reputation: 1062

Your logic looks good but there's always something that can be improved a little further.

If you would like to store only a single user a time in your SocialCard then I would extract only one single user from the API rather than a list because the API returns an array of only one object anyway.

First, I would change the state and include status and error. With status, you can easily check in which status your component is at the moment and based on that render different things/messages in your App. With error, you can define your own error in case something goes wrong and then render an error message in your App. Also, I've re-used your fetch as it was used twice and it was redundant. This way you have a nice single function that can be used anywhere while also making sure that loading is shown while it fetches the result. I've also used MaterialUI Card component for rendering the user data. You can check how the result looks like here

import React, { useState, useEffect } from "react";
import { makeStyles } from "@material-ui/core/styles";
import Card from "@material-ui/core/Card";
import CardActionArea from "@material-ui/core/CardActionArea";
import CardActions from "@material-ui/core/CardActions";
import CardContent from "@material-ui/core/CardContent";
import CardMedia from "@material-ui/core/CardMedia";
import Button from "@material-ui/core/Button";
import Typography from "@material-ui/core/Typography";

const useStyles = makeStyles({
  root: {
    maxWidth: 345
  }
});

function App() {
  const classes = useStyles();
  const [state, setState] = useState({
    user: {},
    status: "idle",
    error: null
  });
  const { user, status, error } = state;

  const getUser = () => {
    setState((prevState) => ({
      ...prevState,
      status: "loading"
    }));

    fetch("https://randomuser.me/api").then(async (res) => {
      if (res.ok) {
        const data = await res.json();

        setState((prevState) => ({
          ...prevState,
          user: data.results[0],
          status: "processed"
        }));
      } else {
        setState({
          user: {},
          status: "failed",
          error: "Error message"
        });
      }
    });
  };

  useEffect(() => {
    getUser();
  }, []);

  if (status === "loading") {
    return <div>Loading ...</div>;
  }
  if (status === "failed") {
    return <div>{error}</div>;
  }
  if (status === "processed") {
    return (
      <Card className={classes.root}>
        <CardActionArea>
          <CardMedia
            component="img"
            alt="user"
            height="140"
            image={user.picture.large}
            title="user"
          />
          <CardContent>
            <Typography gutterBottom variant="h5" component="h2">
              {user.name.first}
            </Typography>
          </CardContent>
        </CardActionArea>
        <CardActions>
          <Button size="small" color="primary" onClick={getUser}>
            Show another user
          </Button>
        </CardActions>
      </Card>
    );
  } else {
    // Default placeholder
    return <div>hi</div>;
  }
}

export default App;

However, if you would like to store all the users you fetch when clicking the button, I would suggest moving the fetch and state of users into the parent component and leave SocialCard component only for rendering a single user. Then, in the parent component I would ensure that the setState would look something like this in the getUser function

setState((prevState) => ({
  ...prevState,
  users: [...prevState.users, ...data.results], // This merges your previous user objects with new user object
  status: "processed"
}));

This way, you can keep all the users in your parent component and using map you can render each user with your SocialCard component. Take a note that you would need to refactor your components further in order to make this work. I'll leave it as an exercise for you if you want to go this route.

Upvotes: 1

Hithesh kumar
Hithesh kumar

Reputation: 1036

Changes Made

  1. Instead of repeating the same code make a function and call that function in useEffect.
  2. Each child in a list should have a unique "key" prop.
  3. Card from Material UI is used.(I have not focused on styling much XD)
import {
  Button,
  Card,
  CardActions,
  makeStyles,
} from "@material-ui/core";
import React, { useEffect, useState } from "react";

export default function SocialCardsfunction() {
  const [items, setItems] = useState([]);
  const [loading, setLoading] = useState(true);

  const classes = useStyles();

  const fetchPerson = () => {
    fetch("https://randomuser.me/api")
      .then((response) => response.json())
      .then((response) => {
        setTimeout(() => {
          setItems(response.results);
          setLoading(false);
        }, 1000);
      });
  };

  useEffect(() => {
    fetchPerson();
  }, []);

  if (loading) {
    return <div>Loading ...</div>;
  } else {
    return (
      <div>
        {items.map((item, i) => {
          return (
            <div key={i}>
              <Card className={classes.root}>
              <h2>{item.name.first}</h2>
                <img
                  alt="img"
                  src={item.picture.large}
                  className={classes.large}
                />
                <CardActions>
                  <Button onClick={() => fetchPerson()} size="small">Next</Button>
                </CardActions>
              </Card>
           
            </div>
          );
        })}
      </div>
    );
  }
}

const useStyles = makeStyles({
  root: {
    minWidth: 275
  },
  bullet: {
    display: "inline-block",
    margin: "0 2px",
    transform: "scale(0.8)"
  },
  title: {
    fontSize: 14
  },
  pos: {
    marginBottom: 12
  }
});

Upvotes: 1

Related Questions