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

Accessible Accordion with Pure JavaScript

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 couldn't get the <main> element to correctly center horizontally on the screen, it is slightly offset to the right. This is especially noticeable on small screen widths. If anyone knows why this happens I'd be grateful to learn it.

Community feedback

P

@Islandstone89

Posted

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.

Marked as helpful

1

@tenze21

Posted

Is it ok to use the header tags like it is done here for styling purpose because i think the developer used the h3 tags to make the text a bit bold but the contents don't actually seems to be header. Just something that struck me while going through the code. @Islandstone89

0
P

@Islandstone89

Posted

@tenze21 No, you cannot skip headings. You can style it in CSS if needed, but the HTML document needs to have headings in order.

0
Jack Smith 350

@Clonephaze

Posted

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.

Marked as helpful

1

@high-rolls

Posted

@Clonephaze thanks, that's a great idea to use the min function! I should learn more about the core functions in CSS, I only know a few of them.

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