Responsive FAQ accordion card

Community feedback

• 

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  Hey nonamehz, good job completing this challenge. Keep up the good work.

  Your solution looks good however the sentence "What is the maximum file upload size" should be in bold.

  I hope this helps

  Cheers Happy coding 👍

  1
• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hi, Jose! 👋

  Congratulations on completing your first JavaScript challenge! 🎉

  Some feedback on this solution:

  • Accessibility
    • Good job on using main landmark! 👍
    • Don't use header for the card content since it is not a full webpage. This is one chunk of content that all belong together and in a real website would sit with other content.
    • Currently the accordion panels are not accessible by keyboard and screen reader. Use the native HTML accordion instead, details and summary. They are accessible by default.
    • Create a custom :focus-visible styling to any interactive elements (button, links, input, textarea). This will make the users can navigate this website using keyboard (by using Tab key) easily.
    • For any decorative images, each img tag should have empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images. In this case, all images are decorative only.
    • There's no need to uppercase the alternative text for images. I'm afraid that the screen reader will spell them letter by letter instead of reading them as a sentence.
    • Each question should not be a heading for sure. The content below it is too small. You might find it helpful if you think of a heading like a title on a document.
    • Use rem or sometimes em unit instead of px. Using px will not allow the users to control the size of the page based on their needs.
  • Styling
    • Don't limit the height of the body element, it will not allow the users to scroll the page if the page content needs more height. Use min-height instead.
    • There's no need to specify width: 100vw to the .container element. By default, the block element would have full width. Also, remove the overflow: auto;.
    • I would rather make the body element as the flex container to center the .container or the card in the middle of the page. That way, I can remove the section tag inside the .container since it's neither adding meaning nor making the HTML more semantic.

  1440px for desktop layout is too late. It looks like you are using the value from the style-guide.md as your breakpoint. The sizes on the style-guide.md have nothing to do with the media queries. They are telling you that "this is how your website should look like at these screen sizes". As frontend developers, we should keep making your website looks good in between those screen sizes.

  That's it! Hope you find this useful! 😁

  1