3column responsive card component

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Desktop layout looks fine, just needed for the learn more to be more pushed down so that it won't touch the text above it. Site is not responsive at the moment since if you resize the screen's width, the layout is being hidden thus creating horizontal scrollbar. The mobile state looks fine on the other hand.

  Some other suggestions would be:

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

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

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

  • Don't use position: absolute on a large element like the section since this removes an element from the normal flow. if you inspect the layout in dev tools, hover on the body tag, you will notice that it has no height, since it's child, the section is out of the flow. Since you are only using this to center the layout, you could have done it this way. First, remove these stylings on the section:

  position
  top
  left
  transform
  

  Then on the main tag add these:

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

  You also don't need position on the footer, remove those same stylings. This way, you have prevented element from being out of the flow.

  • Also, avoid using vh unit in the height property as this is not consistent. Instead use rem values/unit.
  • Add an extra aria-hidden="true" on those images that are decorative that uses alt="" so that it will be totally hidden for screen-reader users.
  • Only have a single h1 on a site. It would be great to change those headings into something like h2.
  • When making a text uppercase, do not directly type the word in the markup capitalized. Doing this results screen-reader reading the text letter-by-letter and not by word. Instead just use regular lowercase text and just use text-transform: uppercase on it on the css.
  • n a site, always have a single h1. Since there are not visible text that are suitable to be h1, the h1 would be a screen-reader only heading. Meaning it will be hidden visually but present for screen-reader users. On this, the h1 would have like sr-only class and the text-content should describe what is the main-content is all about. The h1 could be placed as the first text inside the main. Have a look at Grace's solution on this one inspect the layout and see the usage of h1 as well the stylings applied to it.
  • For the learn more, you should have not added a div inside the a tag. Use the text directly as the a tag's text-content then just add padding to it instead of width and height since they all have the same text-sizes.
  • Lastly, making the site as responsive as you can^^

  Aside from those, great job again on this one.

  Marked as helpful

  0

  ---

  

  GabFerreira•240

  @Gab-FerreiraPosted about 3 years ago

  Hello @pikamart, you're amazing THANK YOU. I will redo this challenge and do it with all your advices and I hope I will succed. Again thank you and i think you already know that but : by doing this you help a lot some people so continue :D

  1