Completed with ReactJS

Community feedback

• 

  hardy•3,660

  @hardy333Posted about 3 years ago

  Hey, good job but try to make accordion responsive on mobile screen sizes.

  Good Luck.

  4
• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Layout looks great in desktop, though your mobile transition is too early. Right now, at point 1200px I think, you are already showing the mobile state which should not be, since desktop layout could use more of those. The mobile layout however looks great.

  Some other suggestions would be:

  • Avoid using vh unit for the height property as this is not consistent since it only relies on screen's height. Instead, use rem so that it will be consistent for all users.
  • Use the main element as the parent container to wrap all the content. It should be after the #root selector, this way all elements that has content will be inside a landmark element.
  • Each img on the should be hidden since they are all just decorative images. So it would be great for them to use alt="' and an extra aria-hidden="true" attribute on the img tag.
  • I wouldn't use h1 on the FAQ word, instead the h1 would be better if it was a screen-reader only text, defining what is the main content is all about, then the faq would be h2.
  • On the accordion, always use interactive elements for interactive components. Using li alone to handle the event is not really great nor accessible. I know that using and learning react is great, but before you dive into things like this, you must first have a strong fundamentals about html and css. Because the last thing you want to be is a react developer that uses wrong markups and wrong stylings.
  • If you don't want to use button with aria-expanded for the accordions, use details element on it. It is native element that is suited for this kind of components, it is already accessible.
  • Your arrow button uses a wrong aria-label value. To be honest, you don't need to use button for this img. It would be better if this arrow-image is used on the ::after of the accordion element. You don't need to use html element on that one.
  • Lastly, try to make it as responsive as you can. Adjust as well the transition point of the media query.

  Aside from those , great work still on this one.

  0