Responsive Stats Preview Component using Flexbox & BEM

Community feedback

• 

  raya•2,870

  @rayaattaPosted 11 months ago

  Hello Matias👋, congratulations on completing your first FED challenge 🎉

  I have some suggestions you might find useful.

  1 I noticed that you used h2 .since it is the only heading in the document you should replace it with <h1>.

  Here's a quick guide on how to use them:

  The <h1> to <h6> tags are used to define HTML headings. <h1> defines the most important heading. <h6> defines the least important heading. Only use one <h1> per page - this should represent the main heading/subject for the whole page. Also, do not skip heading levels - start with <h1>, then use <h2>, and so on. You can then style them in you css.

  Unlike what most people think, it's not just about the size and weight of the text It is about maintaining a clear and consistent hierarchy through out the document

  2 You used max-width: 465px; On the .c-stats-card This is okay but in case a screen is less than 465px wide then the card`s layout might break. You can make it more responsive by using max-width:min(90%,465px) This means it is 465px but on smaller screens it only covers 90% of the width.

  3 Another minor suggestion I have. Try updating the image's border radius on larger screen sizes

  I hope this helps 🙃

  Your solution is awesome 👏

  Happy coding bro✌️

  Marked as helpful

  1

  ---

  

  P

  Matias•50

  @matiaslagoeviaPosted 11 months ago

  @rayaatta thanks for the comment! Let me know if I'm misinterpreting something below :)

  1. I'm not using an h1 in here because I'm thinking of the card as a component, and there'd be potentially many of these in a site. Under that assumption, the applicable heading level seemed to be h2. In this example, I could've either used a h1 elsewhere or within the card, but it felt most applicable to me at the time that, from the point of view of the card, it would not be a page-wide heading.

  2. On a layout 375px wide, the layout works as intended/as I last checked. In widths as small as 255px, the layout still seems good to me? The layout breaks at around 184px wide, when the picture isn't able to fit in the view as is.

  3. I tried the layout on my laptop, large monitor, and on responsive mode mobile screens in different varieties. Is there any specific size you are referring to?

  0

  

  raya•2,870

  @rayaattaPosted 11 months ago

  about the h1 it should always be included,but you can set display none to it.Screen readers rely on it to inform the blind what the content is about.this is usually given the class "sr-only",but since it is a mockup we can just ignore it for now but try checking the accessibility report..

  About the screen width:I said the layout "might break". This is because when doing this solution you are trying to give the impression that the card is in the center of the screen.When you say max width:465px, the card will always take up the available width without exceeding 465px,but this means on any device below 465px,it doesn't leave any inline space to give the center effect. Which would be solved by using min(90%,465px).

  I hope I understood your question right

  Happy coding 😉@matiaslagoevia

  1

  

  P

  Matias•50

  @matiaslagoeviaPosted 11 months ago

  @rayaatta I didn't know about the h1 stuff you mentioned, that's helpful!

  As for the second paragraph, I believe you may be missing some details about how the centering and sizing is working. Here's the code and the live version. I'm personally using Firefox's responsive mode to check different widths, there may be a difference with what you see because of it?

  If you could show me what you mean it may help me understand what you mean, though from the information I have available and how I'm interpreting your responses, they'd seem inaccurate to what I'm seeing. Again, it may be my own misinterpretation of your messages/explanations, so I'm afraid I'd have to see actual screenshots or code references to remove any doubts I may have.

  Thanks again for the feedback!

  0

  

  raya•2,870

  @rayaattaPosted 11 months ago

  I've checked it again and it is working right because of the margin: 0 1em; I am not sure if I had made a mistake earlier but, probably.

  And it really looks awesome, even the code. As a by the way,do you use lighthouse@matiaslagoevia

  0