Mobile First Stats Preview Card, built with Flex and Grid

Community feedback

• 

  T

  Grace•31,230

  @grace-snowPosted over 1 year ago

  Hi

  I'm afraid the HTML needs to change on this

  The biggest issue is with those 3 stats. Numbers like that make no sense at all as headings - what kind of content would you expect to find navigating to a title of "10k"? It's meaningless. Imagine the content in a plain document without styles. That helps with choosing the most appropriate elements. Instead those 3 stats should be a list with 3 list items

  This is a single component with text and an image. If you're going to use section, it should be wrapping all that content, including the image as it is all one component.

  A more minor issue is it is generally considered bad practice to have empty divs in html. Not only is it confusing to other developers (why is that div there) but it is also less performant serving background images like this. Instead, use the picture element. Alt can be blank to treat it as decorative, as you are doing now, but the code is clearer and the performance is way better because the browser can do it's job and pull down the correct image for the users screen

  Marked as helpful

  0

  ---

  

  T

  Grace•31,230

  @grace-snowPosted over 1 year ago

  Css looks OK but I am concerned at the number of % widths in this. I think you may have a misunderstanding about paddings vs margin.

  Margin is for external space. For example creating vertical space between low level elements in the card like paragraph and list. Padding is for internal spacing. For example creating some space inside the card content half of the component so nothing hits the edges of that box

  Marked as helpful

  0

  

  Mark Lawson•90

  @walkonmars36Posted over 1 year ago

  @grace-snow

  Hi Grace, thank you so much for your review and feedback.

  I have changed the stats content to be an <ul> - makes total sense 🙄

  As this is just a single component that would most likely be a section on a bigger page, through trying to be semantic I wasn't sure what to use to avoid everything being a div. I have opted to now just using <main> to wrap everything.

  I deliberated for so long on what to do with the image, I took on board what you said about empty divs and tried using the <picture> tag, but this has <img> as a requirement. So I went back to the research. I believe that this is a purely decorative image so what I decided on was to use it as a pseudo element, with a comment in the HTML and CSS for good developer practice. I'll reply to your other points below

  0

  

  Mark Lawson•90

  @walkonmars36Posted over 1 year ago

  @grace-snow

  Hello again 😁

  Point taken on my excessive use of % widths, I've gone in and tidied that up, using padding and also removed some margin that was coming through from the pre media query styles.

  A couple of new ideas I had was to use clamp on the padding-right of the stats container, as the items were getting squished at a certain point before snapping back to mobile layout.

  As I'd been using one of those % widths to control the text section, I applied a max-width of 40ch to this. I learnt this tip from a recent blog post from Andy Bell.

  Thanks again for your advice, I've just run my site through Lighthouse and got 100% for accessibility and best practices 🎉

  1

  

  T

  Grace•31,230

  @grace-snowPosted over 1 year ago

  @walkonmars36 this looks really good, well done!

  1

  

  Mark Lawson•90

  @walkonmars36Posted over 1 year ago

  @grace-snow

  Thanks Grace 😊

  0
• 

  IryDev•1,580

  @IryDevPosted over 1 year ago

  Hi, well done for your solution !

  I have a little suggestion to improve your solution :

  The image on the right is not properly centered when the window changes of size.

  You can apply those properties especially background-position:center to make it properly centered.

  .card__image{
  background-image: url(./images/image-header-desktop.jpg);
  background-position: center;
  background-size: cover;
  }
  

  Finally I hope you'll find this helpful and your solution is really good

  Marked as helpful

  0

  ---

  

  Mark Lawson•90

  @walkonmars36Posted over 1 year ago

  @IryCode

  Ah, I missed that, there's always something 😀 . Thanks for your feedback 🙏

  0