Random Quote Generator

Community feedback

• 

  Alex•3,130

  @Alex-Archer-IPosted 5 months ago

  Hi!

  That's a cool work, your code is plain and clear =)

  By the way, why did you decided that async function is better than fetch chain? I mean I used the same approach, but it seems that they are both neat =)

  I suggest you to add an error handler in such functions. For fetch chain it is catch:

  fetch( 'url' )
       .than( Doing something... )
       .catch(error => Doing something useful with the error... )
  

  For async function it could be try {} catch {}

  let res;
  
  try {
      res = await fetch( 'url' );
  } catch (error) {
      Doing something interesting with the error...
  };
  

  And don't forget to display some kind of error message for the user. It is a good UE when the interface reports about an error than just disappear =)

  Guess it's all redundant for a simple challenge like this, but it is a good habit to keep this in mind =)

  1

  ---

  

  grgrnkoo•520

  @grgrnkooPosted 5 months ago

  @Alex-Archer-I hi! thanks for your review! in this app i used functions just because it was my first time applying await functions instead of fetch -> then chain

  but in general, at the moment i prefer function method because it can return some data, which fetch is not capable of. i think now i’m in the experimental stage because it is still a new concept for me.

  and doesn’t matter which method to use, i definitely should display error states. thanks

  1

  

  Alex•3,130

  @Alex-Archer-IPosted 5 months ago

  @grgrnkoo

  Oh, yeah, I totally get you. Experiments are part of the fun. I just love to hear different opinions =)

  And I guess async-await pattern is a bit more flexible. In my solution I have to switch between loading or error messages and quotes, occasionally change classes, so I would still have to use fetch chain inside the function.

  0
• 

  Nathaniel Abolarin•650

  @NaythankikPosted 5 months ago

  Add some hover effect for the dice element

  1

  ---

  

  grgrnkoo•520

  @grgrnkooPosted 5 months ago

  @Naythankik that’s not a part of task, so i just added a cursor: pointer

  also i don’t think it’s necessary as a UI part tbh. it is obvious that a button is interactive even without a hover. hovers are used when you need to highlight an interaction

  0

  

  grgrnkoo•520

  @grgrnkooPosted 5 months ago

  @Naythankik oh. i just checked the task one more time. it includes the hover effect... how could I miss this lol

  even though, I like the look of it without a box-shadow, so I think I'll leave it as it is. thanks for your review!

  0

  

  Nathaniel Abolarin•650

  @NaythankikPosted 5 months ago

  @grgrnkoo It's all good but since it was shown in the active state design from the starter pack, I think you should include it. Remember, It's for user consumption!

  1