HTML5, CSS, Flexbox Stats Preview Card Solution

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Layout in desktop looks great, though the mobile layout needs to make use more of the screen-size. Right now, only users with 360px going down can see the mobile layout, phone resolution is much greater than that. I suggest to properly address this, make use of mobile first approach.

  Akanksha already said some great tips, just going to add some as well:

  • Avoid using height: 100vh on large container like the body tag, this makes the element's height, uses only the remaining viewport/screen's height. Inspect your layout, hover on the body tag, you will notice that it doesn't really capture all the layout. Instead, replace it with min-height: 100vh, this takes full height and expands if it needs to.
  • The purple on the heading tag is missing, check that one out.
  • For the card stats, you could have used ul element to nest those, since those are "list" if information.
  • The word 10k 314 and the other one is not suited to be a heading tag. Use heading tag to phrases which gives overview to what is the section is all about, and those words like 10k, does not give any, use p tag on it.
  • You don't need to use figure on the image, you typically use figure when you want an image to be explicitly visible to all, that is why it uses figcaption.

  Aside from those, great job again on this one.

  Marked as helpful

  2

  ---

  

  WannaBeDev•90

  @GColvillePosted about 3 years ago

  @pikamart

  Hey, thank you for the feedback, really appreciate it. I'll go in and make some amendments :)

  I was wondering about my use of the <figure> element, would it be better to just have the <img> on it's own there or perhaps create a <div> and set it's background property?

  0

  

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  @GColville You could use either of the 2 ways.

  But, if you think that the image on the righ side is meaningful enough to the website, you should use img element with a descriptive alt.

  If the image is not really meaningful and just decorative, then you could use just a div and make the image just a background-image, or just use img element, but the with alt="" as empty.

  1

  

  WannaBeDev•90

  @GColvillePosted about 3 years ago

  @pikamart

  I had another go at the challenge and went for the mobile first approach.

  For some reason, I can't get the overlay to work for the image, when I do add the background-color to the <div> element containing the image it doesn't work.

  I think the rest looks better, but definitely room for improvement.

  Thanks again for your advice!

  0
• 

  Akanksha Yadav•355

  @Akku22janPosted about 3 years ago

  Hi. img{ max-width:100%; height:auto; }

  I hope this should fix your problem. Instead of width:100%, max-width:100% will ensure the width of the image reduces when the viewport width start decreasing. Instead of height:100%, height:auto will adjust height according to width to maintain aspect ratio. In this way, the images will never be stretched or squeezed.

  And keep coding.:)

  Marked as helpful

  0