3-column-preview-card-component

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hi Nassim,

  First of all, Congratulation on completing this challenge and awesome work on this one. I have few suggestions regarding your solution:

  • About <h1>it is recommended not to have more than one h1 on the page. Multiple <h1>tags make using screen readers more difficult, decreasing your site’s accessibility. You can have<h1>visually hidden with sr-only. Then swap those <h1> by <h2>.

  • What would happen when the user click those learn more? In my opinion, clicking those "learn more" would likely trigger navigation not do an action so button elements would not be right. So you should use the <a>. For future use , it's a good habit of specifying the type of the button to avoid any unpredictable bugs.

  • In this challenge, the images are much likely to be decorative. For any decorative images, each img tag should have empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images.

  • Adding rel="noopener" or rel="noreferrer" totarget="_blank"links. When you link to a page on another site using target=”_blank” attribute , you can expose your site to performance and security issues.

  Hopefully this feedback helps.

  Marked as helpful

  1
• 

  Lucas 👾•104,340

  @correlucasPosted over 2 years ago

  👾Hello Nassim, congratulations for your new solution!

  I think you did great using BEM, is a really good way to have a standard CSS and work in teams. Well done.

  Something you can fix in your solution is the aligment, you've used height: 100vh inside the main but its better this goes on body to give it the vertical alignment, see that doing these changes now the alignment works. Just remove from main the 100vh height and you'll see the difference

  Remove it and you'll see that the container now align:

  main {
      /* height: 100vh; */
  }
  

  Hope this helps, happy coding!

  Marked as helpful

  1