Article preview component

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

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

  Kamil Szymański already gave feedback on this one, just going to add some suggestions on this one as well:

  • To be honest, I wouldn't really use h1 on the bold text since it doesn't really look like a section heading that gives information on what those would contain, hence p tag would be fine. But since h1 will be removed, the h1 will only be a screen-reader only h1. Meaning this will be hidden for sighted users and only be visible for screen-reader users. 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 image should be using img tag itself and using the person's name as the alt like alt=""Michelle Appleton"
  • When wrapping a text-content do not just use span to wrap it, use meaningful element like a p tag if it just a regular text or heading tag if it is an heading.
  • The button toggle should have aria-expanded="false" as a default attribute and it will be set to true if the user toggles it and vice-versa.
  • Currently, the dropdown is not totally hidden. Using only opacity it only hides the element visually but it is still being picked up by screen-reader, add visiblity property to properly hide the element.
  • Social media links could be inside ul since those are "list" of links.
  • On the a tag that wraps the social media, lose the word "link" since a tag is already a link so no need to use it.
  • Each svg inside the social media link should be hidden since they are only decoration so use aria-hidden="true" attribute on them.

  Aside from those, site looks great.

  Marked as helpful

  0

  ---

  

  Kasia Zaleska•200

  @kzaleskaaPosted about 3 years ago

  @pikamart thanks for every suggestion. I've just fixed my solution based on your tips.

  1
• 

  Kamil Szymański•140

  @szymKamilPosted about 3 years ago

  Nice coding, I'm currently finished this challenge too, but your script is arranged than mine. I'll borrow some ideas from you, line starting from mobile version, instead of computer size of web. I have only one sugesstion in JS coding. Instead of toggling on and off tooltip, try to set that clicking on button with arrow will display tooltip, and clicking outside button and tooltip will hide it. If you want, check my solution: https://github.com/szymKamil/FrontentMentorChallenge5

  Marked as helpful

  0

  ---

  

  Kasia Zaleska•200

  @kzaleskaaPosted about 3 years ago

  @szymKamil thanks for your advice. I've just applied some changes in my JS file and now clicking outside button can hide tooltip.

  0