Responsive Card Component Using CSS Grid, Flexbox, and SASS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Though I had to zoom out because I can't see the desktop layout. Did you code this in large screen? Better test it for the usual 1366x768 screen ratio. Looking at the desktop layout, it is fine, the mobile layout seems fine as well.

  Ammon and Kees already some some great suggestions, just going to add some as well:

  • Lower down your media query breakpoint, right now you are only applying the desktop layout in @media screen and (min-width: 1440px) which is too big for lots of users, that is why they will need to zoom out as well. Lower it down to like 1000px or lesser, or just apply it when the layout really needs to be at the mobile layout.
  • The main element does not need role="main" since it is already a main tag.
  • When using section element, it requires a heading tag inside it. The section element that holds the img, it could be replaced with just div.
  • Lastly, properly center the element. What you could do is, on the body tag, add css like:

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

  Also, this would really help you in the long run. Always apply:

  html {
    box-sizing: border-box;
  }
  
  *,
  *::before,
  *::after{
  box-sizing: inherit;
  }
  

  This makes padding and margin not add any to the current size of the element. This makes your element easy to handle.

  Aside from those, great work.

  Marked as helpful

  0

  ---

  

  Rachel Cessna•195

  @cessnar516Posted about 3 years ago

  @pikamart thanks so much for the feedback! I really appreciate it. The box-sizing and suggested CSS for the body element made a big difference when I applied them to my project.

  I used the desktop screen size specified in the style guide for the challenge, but I agree it's really large. I will adjust it down so the desktop layout shows up for smaller screens. Thanks again!

  0
• 

  Kees den Hollander•140

  @cornelisgijsbertPosted about 3 years ago

  Hi, For color overlay try "mix-blend-mode" as property seen on https://www.w3schools.com

  Marked as helpful

  0

  ---

  

  Rachel Cessna•195

  @cessnar516Posted about 3 years ago

  @cornelisgijsbert That was exactly what I was looking for! Thanks so much!

  0
• Account deleted

  Very nice ! Try to fix the footer at the bottom of the page, with some:

  footer{ text-align: center; bottom: 0; position: absolute; width: 100%; }

  Best regards, Ammon :)

  Marked as helpful

  0

  ---

  

  Rachel Cessna•195

  @cessnar516Posted about 3 years ago

  @ammonschiffer I agree - moving the footer to the bottom of the page makes sense so it doesn't distract from the rest of the page. Thanks so much for the feedback!

  1