Completed with ReactJS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey awesome work on this one. Layout looks great in desktop, it is responsive but I find it weird that the image makes this funky sizes when screen-size changes, its kinda funny ( on my side hahaha), the mobile layout looks great as well.

  Agata already gave great feedback for this one, just going to add some suggestions as well:

  • Avoid using height: 100vh on a large container especially the body tag. This makes the element's height limited to the current remaining screen/viewport's height. Instead use min-height: 100v, this takes full height but lets your element expands if it needs to.
  • Always have a main element to wrap the main content of the page. This helps to organize properly your content and makes users to properly navigate your site.
  • Always have an h1 element on a page. For this one, the h1 would be better if it is a screen-reader only text, meaning it will be hidden using sr-only class, this will be inside the main element.
  • It would be great to add a p tag to wrap the text inside the header element.
  • Name of the person is great with heading tag, but your usage is incorrect. When using heading tag, if you use h4 make sure that h1, h2, h3 are present before it. Using heading tag level incrementally by 1.
  • The social media links should be using a tag for each link, and the whole social media links should be inside a ul element, since those are "list" of links.
  • Each a tag that wraps the social-media icon should have either aria-label attribute or an sr-only element inside the a tag, the value for whatever method should be the name of the social media it wraps. This way, users will know where this links would take them.
  • You should add aria-hidden="true"on the svg for each social media. Since they are only decoration, hiding them would be really great.

  Aside from those, great work again on this one.

  Marked as helpful

  0

  ---

  

  P

  Kevin Davis•280

  @webdev1kevPosted about 3 years ago

  @pikamart Thanks very helpful! I will definitely apply those suggestions!

  0
• 

  Agata Liberska•3,995

  @AgataLiberskaPosted about 3 years ago

  Hi Kevin, nice work!

  The only thing I noticed is that on really small screens, there is a gap between the image and the text-content - something caused by the grid rows.

  Also, it would be good if the clickable element was actually a <button>, with cursor set to pointer and some focus and hover styles. It would be good if you could add an aria-label as well, it's a good practice whenever links and buttons with no text content are used :)

  Hope this helps!

  Marked as helpful

  0

  ---

  

  P

  Kevin Davis•280

  @webdev1kevPosted about 3 years ago

  @AgataLiberska Thanks for the feedback! Really appreciate it!

  0