Accessible Accordion with Pure JavaScript

Community feedback

• 

  P

  Øystein Håberg•13,280

  @Islandstone89

  Posted over 1 year ago

  It looks OK for me, too. But I don't recommend setting a fixed width, that usually does more harm than good. So I would at least change width to max-width - this will allow it to shrink if needed.

  A few other things I noticed...

  HTML:

  • Headings must always be in order, so you should not jump from <h1> to <h3>. Hence, I would change all instances of <h3> into <h2>.

  • Footer text must be wrapped in a <p>.

  CSS:

  • You don't need to give the body a width of 100vw, as that is its default width anyway.

  • Similarly, the default root font size is 16px, so no need to declare it. Remember, though, font-size must never be in px. This is bad for accessibility, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead.

  • It is common to do mobile styles first, as its layout is usually less complex. This means setting min-width media queries, and they should be in rem.

  • You are giving main a box-sizing: border-box on smaller screens. box-sizing: border-box is usually set on all elements, like this:

  *,
  *::before,
  *::after {
    box-sizing: border-box;
  }
  

  This is usually done at the top of the stylesheet, as part of a CSS Reset. I would recommend including it every time - I use this CSS Reset by Andy Bell.

  12

  Marked as helpful
• 

  Jack Smith•350

  @Clonephaze

  Posted over 1 year ago

  Solution looks good! I looked at the page, played around with it in responsive design mode at various sizes, and looked at your code for centering. Everything seems fine, on my screens everything is properly centered. Maybe a scrollbar was popping up and shifting things?

  My one recommendation would be to set your width to something more like "width min(350px, 90vw)". min() is a fun one, allowing contents to adjust for different conditions. It was always choose the smallest value to work with, in this case it would allow your main content to take up to 90 percent of the width of the screen but never appear smaller than 350px. Responsive widths like that are great for designing content that can adapt to various screen sizes without needing to specify widths at specific viewport sizes.

  11

  Marked as helpful