Article Preview w/ HTML, CSS, Vanilla JS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Desktop layout looks great, site is responsive and the mobile state looks great as well.

  I haven't tackled this challenge yet, but for suggestion, here are some:

  • Avoid using height: 100vh on a large container like the body as this makes the element's height capped based on the viewport/screen's height. Instead use min-height: 100vh so that the element will expand if it needs to.
  • The bold text is not really a heading on this one, because if you read it, it is just an opening text for example, like a blog right. Instead for this one, the h1 would be better to be a screen-reader only h1.Have a look at this snippet of mine about sr-only heading
  • Use only the person's name as the alt value.
  • When using img tag, you don't need to add words that relates to "graphic" such as "profile picture" and others, since img is already an image so no need to describe it as one.
  • The button should have a default attribute of aria-expanded="false" which then be set to true by js, if the user toggles it and vice-versa. Use .setAttribute();
  • The svg inside the button is only decorative image so use aria-hidden="true"` on it.
  • Those social-media links could 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 sr-only text inside it, defining where the link would take them. For example, you should use facebook as the value if the link would take the user to facebook.
  • Social-media image should be hidden since it is only a decorative image so use alt="" and aria-hidden="true".

  Aside from those, great job again on this one.

  Marked as helpful

  0
• 

  Jesse Ma•95

  @jma26Posted about 3 years ago

  Thanks for the feedback! I missed big time on the accessibility attributes...lol I guess that's what I get without checking.

  1