3-column preview card component using SCSS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

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

  Some suggestions would be:

  • 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.
  • Don't position: absolute the footer since it just goes in the way when opening up dev tools at the bottom. Remove it then apply flex-direction: column in the body tag.
  • Those car icons are only decoration. Decorative images should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if the image is using svg.
  • Only use descriptive alt on images that are meaningful and adds content to the site otherwise hide the image for screen-reader users.
  • When using img tag, you don't need to add words that relates to "graphic" such as "icon" and others, since img is already an image so no need to describe it as one.
  • Don't need to add header and footer on each article though but I see that you're just using best practice in article :> But remember that landmarks such as article section needs to have a heading tag inside it as its text. On this, the header wraps the heading which is of course not for the article. So maybe using div for this one would be better^^
  • Also, I wouldn't use the h1 as the title car on this one and instead use a screen-reader only h1. Have a look at Grace's solution on this challenge. Inspect the usage of the h1 on this as well the stylings that are applied on it.
  • When wrapping up a text-content, make sure that it is inside a meaningful element like p tag or heading tag and not using like div to wrap the text.

  Aside from those, great job again on this one.

  Marked as helpful

  0
• 

  Adrian•190

  @ohsitePosted almost 3 years ago

  Hi @pikamart, Thank you for your support! As for the body min-height that's exactly what I've been missing! Giving it just height on mobile the content would overlap top and bottom but stay centered. Really big thx for this solution. As for the footer I position it relative to take it out of the DOM order. Making it child of flex container therefore occupying some space in it, will make the second element not centered. Unless....I maybe give a footer margin top auto...yea that might work! As for the screen readers, semantic tags, and arias that is a definitely my weak side, that I have to work on more. If you be so kind please follow my future challanges and share your feedback! I've learned a lot from you. More than form few hours course! Thx again, peace <3

  1

  ---

  

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  @ohsite Hey, really great that you find it useful^^

  Sure, i'll give you a follow so that your projects could appear in my feed! Looking forwards to those challenges^^

  Marked as helpful

  1