Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

dynamic FAQ accordion using javascript conditional statements.

@tenze21

Desktop design screenshot for the FAQ accordion coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


I feel like I could have implemented the dynamics in javascript in a much simpler way but i couldn't make it work. so I would appreciate your suggestions.

Community feedback

T
Grace 29,310

@grace-snow

Posted

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

@tenze21

Posted

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

0
Bruce B 505

@bbsmoothdev

Posted

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

@tenze21

Posted

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

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord