Order Summary Component with React and vanilla CSS (updated)

Community feedback

• 

  Raymart Pamplona•16,040

  @pikapikamart

  Posted over 3 years ago

  Hey, really nice work on this one. The overall layout on this looks really great. Also, congrats on your first challenge here at FEM! Hope that you found this one exciting to create.

  Here are some suggestions for the site:

  • It would be really nice to include:

  *,
  *::before,
  *::after {
    box-sizing: inherit
  

  On your css reset so that each element's sizing will be much easier to handle.

  • Always have a main element to wrap the main content of the site. For this use main tag on the .card selector.
  • The card image on this one, if you look at it, it doesn't really give much information about it since it is only acting as a decoration on this. Decorative images should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if you are using svg instead of img tag.
  • Also, when using img tag, you don't need to add words that relates to "graphic" such as "logo" and others, since img is already an image so no need to describe it as one.
  • For this one, since you didn't use h1 for the order summary text, you should use the h1 as a screen-reader only heading tag. Have a look at Vanza's solution on this same challenge. Inspect the usage of the h1 on this as well the stylings applied.
  • The music-icon svg is only acting as decorative as well, so adding an extra aria-hidden="true" attribute on the svg would be nice so that it will be skipped by screen-reader.
  • Use h2 instead of h4 for the annual plan text. When using a heading tag, make sure you are not skipping a level. If you use h4 then make sure that h1, h2, h3 are all present.
  • On mobile state, at around 350px, the padding on the .cardContent makes the layout's size not respond on the screen-size changes. Adjusting it would be nice.

  Aside from those, great job again on this one.

  11

  Marked as helpful