Advice Generator - I used html5 css, and vanilla js.

Community feedback

• 

  WandoCode•840

  @WandolePosted over 2 years ago

  Hey, I'm looking at your HTML/CSS, and JS file as you asked for async/await. I will wrote a lot of things, but they are almost details and tips to make your dev life easier;-)

  JS script

  • In the HTML, you should give a specific class name (or an ID) for the span for the advice id, and the same for the blockquote. Then, in your JS, you can use document.getElementsByClassName(<CLASSNAME>) (or document.getElementsByID(<ID>)). This way, you don't have to use multiple selectors. Just be carefull if you use the class name: getElementsByClassName will return an array with all the element with the given class.
  • Instead of setting default value to not start with an empty card, you can load a first advice when the HTML is loaded. To do that, just execute your getRandomAdvice function once at the end of your JS script and fill adviceID and advice. Or even better: write a function to fill them, so you can call it inside of the event listener CB function too!
  • you can use async for the CB function of your event listener too, I think. Just write ..., async (e) => { .... This way you can use await getRandomAdvice() inside of the CB function.
  • It's a good practice to use try...catch with async function when you fetch. Here is an intersting link about fetch HTTP Response in JavaScript: https://bobbyhadz.com/blog/javascript-get-response-status-code-fetch (You will see that response.ok == true when the status is 2xx).

  Design

  • the separator should not move depending of the size of the advice. Instead, .container should adapt his size. Try to play with justify-content: space-between or flex-endfor example, and look at the next comment about with and height.
  • You should not give a defined width and height for element that you want to be modular or responsive. Prefer relative units like width: 50% of flex-basis: 50%
  • I was looking for information about em and rem units because I use mostly the 1st one. I found a very good text about it (and learned how to use them more efficiently), so I shar it with you: https://zellwk.com/blog/rem-vs-em/
  • For the dice inside of the button, just put an img tag inside of a button tag.
  • To move the button more easily with position: absolute (that's good), try to move the button in the .container div directely (not inside bottom-border-style). You put position: relative in that div.

  You can look my implementation of that challenge to help you, I use a lot of what I talked about. It's not perfect but it should help a bit (I need to change my use of em and I have to use try ... catch for my fetch, for example)!

  Marked as helpful

  1

  ---

  

  JWord•140

  @jwren4170Posted over 2 years ago

  @Wandole Thank you. I really like your idea of gettingRandomAdvice instead of hard coding it. I didn't even think of that. Yeah my flex stuff really needs work, I knew that would get some constructive criticism. Actually it's all good advice, no pun intended.

  The reason I use mostly rem units though is because it is base of the default font size of the browser, usually 16px. I know ems can be helpful, but there is also the problem of compounding with them as they are based on the containers font size. At least that is my understanding of that.

  Again thank you :-) I am going to try new challenges and then get back to this one later. It was my first attempt at anything like this as you can probably tell.

  1