HTML , CSS and JS page with media queries

Community feedback

• 

  Aman Singh Bhogal•1,010

  @asbhogalPosted over 1 year ago

  Hi Nawal,

  This is a good solution, well done! Just noted a few things below:

  • Avoid using var when declaring variables. It's best to use let for those which are redeclared later, or const for those that aren't. In this case, you can use const.
  • Variable names should also be in camel case (meaning, the first letter is always lowercase and the first letter of the next word in it is uppercase, e.g. first and firstNumber) and here they should be more clearer (to help other developers identify exactly their purpose, which becomes important especially as your code scales, e.g. instead of Icon, use hamburgerToggle and instead of Show use mobileNav)
  • It's better to let CSS handle the visibility and styling of elements rather than manipulate them in JS (for performance reasons). You can refactor the code to introduce a specific class to display the element as a block and the use the toggle() function in JS to show this via .classList.toggle() on the icon event listener (ie. hamburgerToggle.addEventListener("click", function () { mobileNav.classList.toggle("show"); });
  • It's great you've locally hosted your font, however this should be in woff2 format as this is designed for the web. Here's a link from W3 explaining in further detail Link

  Hope this helps!

  Marked as helpful

  0