Advice Generator App Using React

Hi @Robincredible,

You did a good job for this challenge. 👍 I noticed that you have approached this React project with functional components. I really appreciate the animations you have added. 👌 Below are a few things that I have noticed and you might want to check in order to improve your solution.

• I think if you went with you initial thought of generating a random id to get the advice, it would have resulted in a simpler solution in terms of the number of states to manage. After checking the documentation of the adviceslip api, I saw that the random advice endpoint is cached. So you had to come up with a logic to ignore a click within that 2 seconds delay. By moving all the logic in the <Advice /> component itself, we get the following result.

import React, { useEffect, useState, Fragment } from 'react';
import mobileDivider from '../images/pattern-divider-mobile.svg';
import divider from '../images/pattern-divider-desktop.svg';
import dice from '../images/icon-dice.svg';

const Advice = () => {
  const [ slip, setSlip ] = useState(null);
  const [ isLoading, setIsLoading ] = useState(false);
  const [ error, setError ] = useState(null);
  let adviceContent;

  const getSlip = async () => {
    const randomId = Math.floor(Math.random() * 200) + 1;

    try {
      setIsLoading(true);
      const response = await fetch(`https://api.adviceslip.com/advice/${randomId}`);

      if (response.ok && response.status === 200) {
        const data = await response.json();

        if (data.hasOwnProperty('slip')) {
          setSlip(data.slip);
        } else {
          setError(data.message.text);
          throw new Error(data.message.text);
        }
      } else {
        const errorData = await response.json();
        throw new Error(errorData);
      }
    } catch (error) {
      setError(error.message);
    } finally {
      setIsLoading(false);
    }
  };

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

  if (isLoading) {
    adviceContent = (
      <Fragment>
        <h1>Loading</h1>
        <Loader />
      </Fragment>
    );
  } else if (error) {
    adviceContent = (
      <Fragment>
        <h1>Sorry</h1>
        <p>{ error }</p>
      </Fragment>
    );
  } else {
    adviceContent = (
      <Fragment>
        <h1>{ `Advice #${slip?.id}` }</h1>
        <p>{ slip?.advice }</p>
      </Fragment>
    );
  }

  return (
    <div className="wrapper">
      <div className="loader-wrapper">
        <div className="loader noselect"></div>
      </div>
      <div className="advice-wrapper">
        <div className="quote">{ adviceContent }</div>

        <div className="divider noselect">
          <img src={mobileDivider} srcSet={`${mobileDivider} 550w, ${divider} 1920w`}  sizes="(max-width: 600px) 550px,
            1920px" alt=""/>
        </div>
      </div>
      <button type="button" className="random-quote-button noselect" onClick={ getSlip }>
        <img src={dice} alt="Random Quote" />
      </button>
    </div>
  );
};

//pure CSS animation from https://codepen.io/sudeepgumaste/pen/abdrorB
const Loader = () => {
  return(
    <div className="loading-box">
      <div className="loading-container">
        <span className="circle"></span>
        <span className="circle"></span>
        <span className="circle"></span>
        <span className="circle"></span>
      </div>
    </div>
  )
}

export default Advice;

I think by doing so, prevents you from scattering the logic to achieve a goal into several components which can unnecessarily complicate things.

Ideally, the <Loader /> component would have been in a separate file as well 😉

• It is also a good practice to catch potential errors when dealing with asynchronous operations. In the proposed refactored <Advice /> component, I have used a try...catch block with async...await but you could do same if prefer the promise approach.

I hope this helps.

Keep it up.