[2nd] Pricing Cards w/ toggle btn, Mobile First, javascript

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted over 3 years ago

  Hi you need to use radio inputs for that toggle, not a checkbox. It's inaccessible as it is at the moment.

  It also needs focus-visible styles

  The reason your js isn't working is because querySelectorAll returns an array, but you are trying to use a method for a single dom node. You need to do the classist toggle in a loop instead.

  Something like this:

  function togglePrices(array) {
    array.forEach(item => item.classList.toggle("hidden"));
  }
  
  slider.addEventListener("click", function () {
    togglePrices(monthly);
    togglePrices(annual);
  });
  

  Other best practices:

  • use hidden attribute instead of a hidden class if all it's doing is display none
  • target selectors using js-specific classes, such as js-monthly-price etc so there can never be conflict over styling vs javascript. Another developer working in HTML/css may change a class, not realising it is used in javascript.
  • if you do need to use a state class, try using prefixes like is. e.g. is-hidden or is-active or is-open etc
  • keep event listeners and functions they run separate

  Marked as helpful

  1

  ---

  

  Yuko Horita•645

  @Sloth247Posted over 3 years ago

  @grace-snow Hi Grace, thank you very much for your feedback as always. I will try your solution along with learning js more.

  0