3-column preview card component - css and html

Community feedback

• 

  Lucas 👾•104,400

  @correlucasPosted about 2 years ago

  👾Hello Jessy! Congratulations for your solution!

  I've to say you that you've done a great job for your first time using flexbox. Thats really nice.

  I've some tips for you but are small details, the hard work, you've already done and thats fine. You can use flexbox to align the component by the body, all you need to do is replace the height with min-heigh: 100vh to make the component align to its parent (the body). See the changes I did to you code below:

  }
  body {
      display: flex;
      min-height: 100vh;
      /* height: 100vh; */
      /* margin-left: 300px; */
      align-items: center;
      justify-content: center;
  }
  

  About the cards, you can replace the div with an article to improve the semantics, and use a general class to manage the 3 cards together and on single card to manage the different aspects, like the colors. This way gets easier to change stuff without touch every class separately.

  Hope it helps and happy coding!

  0
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted about 2 years ago

  Hello Jessie Fabian,

  Excellent work! Congratulation on completing this challenge. I have some suggestions regarding your solution if you don't mind:

  HTML

  • Page should have one level heading. You can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech).

  • 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 .

  • 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.

  • You have used <br> , using <br> is not only bad practice, it is problematic for people who navigate with the aid of screen reading technology. Screen readers may announce the presence of the element. This can be a confusing and frustrating experience for the person using the screen reader. You can read more in MDN

  • It’s not recommended to use <br> to increase the gap between lines of text Or make empty space between elements, use padding / margin styling via CSS. And about using <br> in the <p> to make the paragraph break in new line, You may use max-width to <p> and remove those <br>

  There are so many repeated style rules , better to use reusable and manageable classes. For example: each column have the same styles , So you can use a class .card for the shared styles, then for each distinct styles like (background color) use other classes . card__sedans, . card__suvs and .card__lux

  • line-height: 25px; Use a unitless line-height value to Avoid unexpected results. You can read more in mdn

  • Check the component responsiveness again.

  Overall, Your solution is great. Hopefully this feedback helps.

  0