Card component using SCSS and BEM

Community feedback

• 

  P

  Dave•5,295

  @dwhensonPosted over 3 years ago

  Hey @alejandroAyl - really lovely job here! 🙌 Not much I can add or suggest to be honest. The component responds really nicely and everything is lined up perfectly.

  Some very, very small suggestions you might like to think about, but I'm really down in the details here:

  1. I'm not sure about the use of the all the article tags you have included. In this case I would have probably just gone for a plain old div for the parent element, or perhaps considered an ul with the lis as flex-children. This is debatable I'm sure.

  2. Another small thing is to consider styling the outline property - when tabbing through the page this would give the links a nice color when they are focused rather than the default blue.

  Some standard code I use in most projects for this is:

  :focus-visible {
  	outline: 3px dotted currentColor;
  	outline-offset: 0.25rem;
  }
  
  :focus:not(:focus-visible) {
  	outline: transparent;
  }
  

  If you drop that in at the top level that should take care of things - maybe give it a try... The only thing to remember is that if we remove the default outline we must add back an alternative for accessibility reasons (which is what we've done here).

  As I said though, these are very small points and don't detract at all from the great job you did here! 👍

  Keep up the good work! Cheers 👋

  1

  ---

  

  Alejandro Ayala•115

  @alejandroAylPosted over 3 years ago

  @dwhenson Thanks a lot! Very valuable suggestions. There is always room for improvement.

  0