dynamic FAQ accordion using javascript conditional statements.

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted 11 months ago

  As I said on discord:

  In HTML need to

  • remove the header. The header landmark is for site-wide repeating primary content like logo and nav. It should not have headings or page-specific content inside
  • add aria-hidden="true" focusable="false" to decorative svgs. Or use the img element with empty alt
  • use button elements with aria-expanded for the disclosure triggers. I see you've already been given feedback on this and sent my article on it. These changes are essential and it needs to be the whole question not just a decorative icon inside the button.
  • use a footer landmark for the attribution
  • import fonts in the html head not css imports as it's slightly better for performance
  • all the question wrappers and all buttons should have the same class. You should not be selecting each question individually in JS. You need to loop over all of the buttons and toggle the aria-expanded value. That's all this challenge needs. Everything to do with showing and hiding can be handled in css based off that aria-expanded value. Alternatively you can toggle the aria-expanded value and toggle a class on the button like is-open and use that as the css selector to show the answer.you shouldn't be changing any inline styles in JS or selecting each item individually.

  in css the advice is standard:

  • use a css reset at the start of the styles
  • don't use a width on the grid, use max-width so it can shrink narrower if and when needed
  • cursor pointer should only be on interactive html elements.
  • media queries should be defined in rem or em not px
  • work mobile first. That means you should have mobile styles as the default and use a min-width media query for larger screens to change only the properties that need to change for the new layout
  • you will need to use selectors to show/hide answers. Likely these would be sibling combinators like

  .accordion-btn[aria-expanded="true"] + .answer {
      display: block;
  }
  
  // or using the class method
  .accordion .is-open + .answer {
      display: block;
  }
  

  Marked as helpful

  1

  ---

  

  Tenzin Choeda•210

  @tenze21Posted 11 months ago

  Thanks a lot @grace-snow I guess there are a lot of stuff I really need to explore...

  0
• 

  Bruce B•505

  @bbsmoothdevPosted 11 months ago

  Some accessibility help. You shouldn't add event handlers to non-functional elements because keyboard users won't be able to access them. The element that shows/hides the content needs to be functional. I would suggest a button. See Tutorial: Let's Build an Accessible Disclosure for an example of how to do this.

  Marked as helpful

  0

  ---

  

  Tenzin Choeda•210

  @tenze21Posted 11 months ago

  Thanks for the feedback will improve on it. Also thanks a lot for the meterial@bbsmoothdev

  0