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
@tenze21
Posted
Thanks a lot @grace-snow I guess there are a lot of stuff I really need to explore...