advice-generator-app

Community feedback

• 

  Segni Adeba•190

  @SegniAdebaGodsSonPosted over 2 years ago

  Looking good, I got around multiple clicks/fetches by adding a loading state for the fetch API in JS so that while it's in the loading state I made the dice button disabled to click. I used something like this:

  const useFetchAdvice = (url: string, rerun: boolean) => {
      const [advice, setAdvice] = useState<state>({
          data: null,
          loading: true,
          error: null
      });
  
      useEffect(() => {
          setAdvice({
              data: null,
              loading: true,
              error: null
          });
  
          fetch(url)
          .then(res => res.json())
          .then(dat => setAdvice({
              data: dat,
              loading: false,
              error: null
          }))
          .catch(err => setAdvice({
              data: null,
              loading: false,
              error: err
          }))
      }, [...])
  
  }
  

  2

  ---

  

  Manuel Gil•340

  @ManuGil22Posted over 2 years ago

  @SegniAdebaGodsSon Ohh i see, and how do u make the button disabled? I assume u are using a button tag right? Im using a div with the dice icon inside and not sure how to make it like a disabled button. I mean, i can do 'cursor: wait' but its still clickable. I might have to do a button 🤔

  1

  

  Segni Adeba•190

  @SegniAdebaGodsSonPosted over 2 years ago

  @ManuGil22 I've had a feedback under my solution that the clickable dice should be in a button tag for accessibility reasons I believe, I used a div just like yourself and changed it to button. If you use a button you can use the 'disabled' property, so then you can easily set it like 'disabled={loading}'...or if you stick to using div tag you can fetch conditionally, only fetch onClick if loading state is false.

  Marked as helpful

  0

  

  Manuel Gil•340

  @ManuGil22Posted over 2 years ago

  @SegniAdebaGodsSon Alr! Thanks!!

  1
• 

  Danilo Blas•6,300

  @Sdann26Posted over 2 years ago

  Hi Manuel!

  Aside from the above comment I would recommend that you put a lot of emphasis on the theme of the report generated for this challenge.

  You have used images without the alt="" attribute which is very necessary, in case it is an image that is only decorative you can add to the img tag the alt="" and aria-hidden "true" attributes. And in case it is an image that is not decorative but illustrative, you must add alt="Insert your text here".

  Also I would recommend you to change the <div class="App"> to <main class="App"> since you need to have at least one main per page that contains the main content of the page. I would also tell you to add a <h1> which is also missing or change it in the Advice.

  After this generate a new report to see what errors it has.

  Good Luck!

  1

  ---

  

  Manuel Gil•340

  @ManuGil22Posted over 2 years ago

  @Sdann26 Hey Danilo, thanks for the feedback! Will change those things rn!

  0

  

  Danilo Blas•6,300

  @Sdann26Posted over 2 years ago

  @ManuGil22 You're welcome :D!

  0