Three Column, HTML and CSS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, awesome work on this one and congrats as well for your first challenge here at FEM! Hope you had fun doing this challenge and learned from it:>. For desktop layout, it just needed it to be centered and it's good to go.

  Here are some other suggestions for the site:

  • It would be great to have a base styling of this:

  html {
    box-sizing: border-box;
    font-size: 100%;
  }
  
  body {
   margin: 0;
  }
  
  *,
  *::before,
  *::after {
    box-sizing: inherit
  }
  

  This way, handling an element specially its size will be easier because of the box-sizing

  • In a webpage, your main-content should be wrapped inside the main tag so that it will be easy for the user to know and navigate on the main landmark. For this one, create a main tag that will wrap the 3 card.
  • Normally, I would suggest using a flexbox to center the layout on this challenge, but since there is not container that wraps the 3 card, using flexbox on the body will just ruin it. But if you manage to create a main, then use:

  align-items: center;
  display: flex;
  flex-direction: column;
  justify-content: center;
  min-height: 100vh;
  

  Styling on the body tag.

  • On each of the div card, instead of height use min-height. Using height will give an explicit /fixed sizing on the element making it not scale base on it's content, while min-height will allow the element to resize if needed.
  • Also, you could have create a single selector like .card that you can give on each of the card on the layout, this way you won't have to manually add the stylings on each of the sedan, suv, luxury selector.
  • For each of the car icons, they are only acting as decorative images. Decorative images are just images that doesn't contribute to the overall content of the site. They 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 you are using svg instead of img tag.
  • In any webpage, you should have a single h1 tag, usually it is the first heading tag inside the main tag. It describes the content of the site, but since there is no visible heading text on this one, you should create the h1 as screen-reader only h1. Have a look at this solution of Grace on the same challenge. Inspect the website and see how she used the h1 and the stylings applied to it.
  • Those "learn more" are better using a tag rather than button because on a real site, those would be page links for the user to visit and "learn more" about a specific car.

  Aside from those, great job again on this one.

  Marked as helpful

  2

  ---

  

  Jason Haygood•40

  @jkhaygoodPosted almost 3 years ago

  @pikapikamart I really appreciate the feedback! I have been doing online course and wanted to step away and try something on my own.

  Im going to try what you said!

  1