Advice Generator using API and React

Community feedback

• 

  Mi Vu•380

  @gerichilliPosted over 2 years ago

  Hi Alice, you did a great job. However, I think you can make it better at some points.

  • Dice button: You can make the button glow effect look nicer by increasing the blur-radius of box-shadow by a very large number. For example:

  .Dice-button:hover {
      box-shadow: 0 0 60px 5px #53ffaa;
  }
  

  The exact number in the design is box-shadow: 0 0 64px #53ffaa; (spread-radius is 0)

  • fetchAdvice: I think fetchAdvice is an asynchronous function, so you need to use async keyword before ()

  const fetchAdvice = async () => {
   };
  

  In my opinion the next data update will depend on the network speed and the API we are using. data can be returned only when the fetch is successful and I read at https://api.adviceslip.com/ that Advice is cached for 2 seconds. Any repeat-request within 2 seconds will return the same piece of advice..

  • I also think you can use setAdvice directly for the state without using the adviceSet function and name the properties of the advice object the same as the data returned by the API so they look simpler like:

  const fetchAdvice = async () => {
      fetch('https://api.adviceslip.com/advice')
      .then(response => response.json())
      .then(data => setAdvice({...advice, id: data.slip.id, text: data.slip.advice}));
    };
  

  • Regarding the default page. I think you can use the condition on the advice object, not its properties. So I think you should set advice initialization to null instead of {} because empty object is truthy and it can cause unexpected behaviors in conditional operations.

  const [advice, setAdvice] = useState(null)
  //...
  //...
  return (
    <div className="Card">
      {advice ? (
        <>
          <h4 className="Advice-id">
            Advice # {advice.id || "your default text if advice has no id"}
          </h4>
          <h1 className="Advice-text">{advice.adviceText || "your default text if advice has no text"}</h1>) : (
        <>
          <h4 className="Advice-id">Advice Generator 3000</h4>
          <h1 className="Advice-text">
            Welcome to the Advice Generator. Click the Dice below to start.
          </h1>)}
    </div>
  );
  

  I'm not good at React, if there are any mistakes above, I look forward to discussing them further with you. Anyway, have a nice day and happy coding ^^

  Marked as helpful

  0
• 

  Devmor•470

  @devmor-jPosted over 2 years ago

  Hey, this is good.

  About your question, that delay is by design as the API noted:

  Note: Advice is cached for 2 seconds. Any repeat-request within 2 seconds will return the same piece of advice.

  So you should disable button for 2 sec after each request sent, this way user knows that they have to wait a moment.

  Use setTimeout that would help.

  Marked as helpful

  0
• 

  teamie•470

  @tea-scriptsPosted over 2 years ago

  Hello there Alice.

  When fetching data from an external API no matter how you set up your app there will be a request and this usually happens over time, could be in a few seconds or in ms. Clicking on the button repeatedly would send multiple request to the server and if your network is fast you might get immediate responses but with a short delay either in seconds or ms. You can refer to your network tab in your dev tools in your browser to see how it works.

  I tried your app and sent 20 immediate request and I was able to get the strings but had to wait a few ms after the first 2 request because we're fetching from an external source. If your data was local, perhaps it might be different.

  For me I think your app works fine. An alternative to fast response would be building your apps using Gatsby.

  0