Responsive tip-caluculator-app using SASS, Grid, Flexbox and vanillaJS

Community feedback

• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 3 years ago

  Nice work on this challenge; your solution looks great! I'd recommend reviewing the accessibility and HTML report and the try to resolve those issues.

  Instead of using document.getElementsByClassName and document.getElementById, it's now more common to use document.querySelector and pass in the CSS selector.

  There's not much need for the initial if statement either. Running the ready function when the DOM content is loaded is fine. I'd also avoid using var nowadays and exclusively use const or let, defaulting to const. This will make you much more intentional about how you declare your variables.

  Overall, you've broken up the functions well. Try looking for repetitive blocks of code that could be refactored and reused.

  I hope that helps. Keep up the great work!

  Marked as helpful

  1

  ---

  

  PowerZ•85

  @Pow3rZPosted over 3 years ago

  @mattstuddert I resolved issues from the report right away but had some problem with generating new report. For some time it didn't see changes made and gave me the same issues again.

  Does querySelector have some advantages over getElement methods other than not being restricted to only class and id ?

  From what i understand the initial if statement does just that checks if DOM content is loaded, or waits for it to load. Yeah i guess i have to step up my variables game :)

  Thank you for your feedback and have a great day!

  0

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 3 years ago

  @Pow3rZ, sometimes it can take a minute or two for the live version of your project to update. It looks like you've resolved it now! 🎉

  Using querySelector doesn't necessarily have lots of advantages. It just keeps your code a bit more consistent and provides more flexibility for how you select elements.

  Yeah, you're right about the if statement, but it's not necessary as the ready function will run when the document is loaded anyway. That's the purpose of that event listener. It's not a big deal either way, but it would remove a few lines of unnecessary code 🙂

  1

  

  PowerZ•85

  @Pow3rZPosted over 3 years ago

  @mattstuddert Thank you for your advice ! I found querySelector and querySelectorAll much easier to use. querySelector used on a class returns first instance of that class automatically and querySelectorAll returns array not object like getElementsByClass so it is easier to manipulate like i.e. you can use forEach method on it by default. Saves some code and makes it easier to read. Thanks again. Cheers

  0

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 3 years ago

  @Pow3rZ, that's great to hear! They definitely make selecting and working with elements in JS much easier! 🚀

  0