Completed with Plain JS and CSS

Community feedback

• 

  Ejovi Okiekpakpo•345

  @ruona88Posted about 3 years ago

  This is the most detailed review of my work ever, thanks.

  0
• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Layout in desktop looks great, it is responsive and the mobile state looks great as well.

  Some suggestions would be:

  • First, there is an error in the console, you might want to check that one out.
  • Right now, your body tag has no height because it's direct child the main is using position: absolute. Avoid using position: absolute on a large element since it makes the element out of the flow. If you are just centering it, remove the position props and on the body just add:

  align-items: center;
  display: flex;
  flex-direction: column;
  justify-content: center;
  min-height: 100vh;
  

  • It would be better if the left-side part of the layout are both being wrapped in a parent div and the right-side which contains the 6 cards are wrapped as well on their own parent element so that your markup will be more clearer since elements are isolated on their own.
  • A page must have a single h1 on a page. Since there are no text-content that are visible that could be h1, you will make the h1 screen-reader only text. Meaning this will be hidden for sighted users and only be visible for screen-reader users, search about sr-only stylings and see how it is used. The h1 text should describe what is the main content is all about, this h1 would be placed as the first text-content inside the main element.
  • Person's img should be using the person's name as the alt like alt="Jeremy Robson". A component like this when a person's name and image are both present, use the person's name as the value as it is a meaningful image.
  • Name of the person could use a heading tag.
  • Those list of selections shouldn't use a link a tag because those aren't links, use button instead because it is a control. But using button alone is not accessible that is why you need to make use of aria-live element that announces a certain selection is selected and what could be the changes made. You could make use of radio buttons as well on this since it is a selection and radio buttons are great for those. This will be inside a fieldset along with a screen-reader only legend that describes what is the purpose of those set of radio buttons.
  • Each card title like work could use a heading tag since it gives overview on what the section would contain.
  • I think there is an svg for the 3 dots, better using it than creating extra html elements.

  Aside from those, great work again on this one.

  0