Advice generator app

Community feedback

• 

  Joanna Skrzypczak•530

  @joaskrPosted almost 2 years ago

  Hi Tiff,

  Good job with this challenge!

  If you want to further improve your solution here are some tips:

  • To get rid of Buttons must have discernible text accessibility error you can use aria-label on the button. It won't change anything on the screen but will make the button more accessible for users with screen readers. <button aria-label="Get new advice" class="button">. Here you can see more ways to fix that.
  • Consider using rem and em instead of px for font sizes as they are better for responsiveness. Here you can read more about it.
  • I would also recommend removing console.log(data) from the 8th line in app.js file beacuse leaving console.logs are considered a bad practice.

  Overall, great job!

  Keep coding and good luck with future challenges

  0
• 

  Sinisa Vukmirovic•1,690

  @SinisaVukmirovicPosted almost 2 years ago

  Hello!

  Looks solid!

  What I would change is importing Google Fonts in the CSS, not HTML. You are using then and setting "font-family" in the CSS, right? Makes sense to import them there, too.

  Also, try to use more semantic elements in your HTML not just <main> element.

  Hope this helps!

  0

  ---

  

  Joanna Skrzypczak•530

  @joaskrPosted almost 2 years ago

  @SinisaVukmirovic Hi, isn't adding fonts via <link> element to HTML a preferred way to do that? Even on Google Fonts Documentation they use <link> in HTML as the way to include their fonts on the websites.

  I've also found this discussion on Stack Overflow and most of the participants of this discussion believe that using <link> is more beneficial than adding it via @import in CSS. As advantages of such solution they give improved SEO and performance, due to the prefetching of the font.

  0

  

  Sinisa Vukmirovic•1,690

  @SinisaVukmirovicPosted almost 2 years ago

  @joaskr Really? didn't know that about being better for the SEO. Makes sense now that I think about it.

  I thought it makes more sense to do it in the CSS where you are using them.

  Will have to investigate more about it.

  Thank you very much!

  0

  

  Joanna Skrzypczak•530

  @joaskrPosted almost 2 years ago

  @SinisaVukmirovic I wasn't aware either. I started researching this topic after I saw your comment and SEO and performance were mentioned quite often in the discussions. You learn something new every day 😊 Let me know if you find anything else on this topic. It's quite interesting.

  0

  

  Sinisa Vukmirovic•1,690

  @SinisaVukmirovicPosted almost 2 years ago

  @joaskr You learn something new every day, I guess, is the best I could say. Good job for you.

  I didn't find anything different than you.

  Is it better to import fonts in CSS or HTML? Loading Google fonts in the HTML reduces the critical request depth and speeds up page load Always import your fonts from HTML, not CSS.

  Is it better to link or import fonts? It's better you use link if you are loading it from CDN. If the font is in your local directory then loading it using import or link will not have too much significant difference. But if you are loading it using a third party CDN then it's always better to use link .

  I guess its time to stop giving, especially wrong, advices to people and focus on building projects.

  I should get back to the people I gave wrong advice about this, at some point in time. Would you like to be credited and mentioned as a solver of this "dilemma"?

  0

  

  Joanna Skrzypczak•530

  @joaskrPosted almost 2 years ago

  @SinisaVukmirovic Don't worry about it, we learn by making mistakes 😊 And if it wasn't for your comment I wouldn't even start researching the topic!

  Good idea with getting back to other people. You don't have to credit me but thanks for asking 😊

  1