3 column preview card component / flex

Community feedback

• 

  P

  Dave Quah•660

  @MilleusPosted almost 2 years ago

  👋 Hello!

  Congratulations on completing the challenge. 🥳

  There are a few things that I think would make this even better:

  • Use only one <h1> per page, for accessibility and SEO. In this case I'd see "Sedans", "SUVs" and "Luxury" of the same logical level, so I'd approach this by adding a <h1> with a screen reader only class (visually hidden but still picked up by screen readers), and have "Sedans", "SUVs" and "Luxury" as <h2>. Here's a good video by Kevin Powell sharing more about headings.
  • In the footer, to be more semantically correct, use <p> instead of <div> for texts.
  • For the rounded borders, an alternative approach is to set border-radius: 8px and overflow: hidden on the parent. This could make things a little easier as the border-radius would not need to be updated in the media query.
  • For the anchor elements CSS, because the font-size isn't explicitly set and there's a fixed pixel height and width set, there will be text overflowing out of the element when the base font-size increases (i.e. someone who increased font size on their device). Most cases of fixed widths or heights could be replaced with min-width or max-width etc, so that it scales better. Another approach would be to use padding instead. I'd also recommend using em or rem units instead of px for better accessibility.

  Nice work and keep up the good job! I hope this helps and happy coding! 😊

  Marked as helpful

  0

  ---

  

  Magdalena•500

  @magsaramPosted almost 2 years ago

  @Milleus Hi Dave! Thank you for opinion :)

  You're right, there should be one h1, I know it, I use it three times because there was no one one main title, so I was stupid :D But I'll remember now, h2 is better on this kind of projects :)

  And I know, there must be <p> in <div>... I don;t know, why I didn't use it, must be more precise.

  Thanks for hint about overflow-hidden! I know this property, but I forgot it now :/

  Anchor - yep, I understand, I'll try to remember <3

  0