Stats preview card component

Community feedback

• 

  Elaine•11,400

  @elaineleungPosted about 2 years ago

  Hi Cas, well done on another solution! 😊 I'll try to answer some of your questions here:

  1. I think the semantics look fine, and good use of the picture element! The way you've written it is slightly different than how MDN writes it (they have the img containing the image that's in your main code, which in your case should be the mobile version since you did this with a mobile first approach), but as long as the picture switches and everything works, then that's fine.

  2. I generally use rem to handle limiting the width and dealing with responsiveness. Here's something you can try: Instead of a max-width, use width: min(100%, 30rem), and for the desktop view, change that to width: min(100%, 60rem). I also suggest making the breakpoint higher, maybe at 980px, so that when the layout changes, the displayed part of the image can still show the important parts of the image (or alternatively, you can also position the image in a way to do show what you want to show).

  3. About the CSS, I think you did well using a mobile first approach; the only thing I'd probably do differently is to format the code (which can be done easily in VS Code) to get rid of some of the spacing, and I'd also use different class names for some of these selectors. For example, I would call the image container card-image and the text container card-content, just so it's clear that these two are children of the card.

  Last note: I think using viewport widths and percentages for widths can work if you use something like min() (as in, what I did here) where you have other options like a fixed relative width (e.g., 30rem). Using just viewport or percentage widths alone can lead to undesirable outcomes at times if there aren't other properties included.

  Hope some of this feedback can help you out!

  Marked as helpful

  1
• 

  Lucas 👾•104,340

  @correlucasPosted about 2 years ago

  👾Hello @iguanasplit, Congratulations on completing this challenge!

  You’ve done really good work here putting everything together, I’ve some suggestions to improve the design:

  1.Use a CSS reset to avoid all the problems you can have with the default CSS setup, removing all margins, making the images easier to work, see the article below where you can copy and paste this css code cheatsheet: https://piccalil.li/blog/a-modern-css-reset/

  2.I saw that for some properties you’ve used rem and for others px. In this case it is better to use only one kind of unit to have a better organization for your code. relative units as rem or em that have a better fit if you want your site more accessible between different screen sizes and devices. REM and EM does not just apply to font size, but to all sizes as well.

  ✌️ I hope this helps you and happy coding!

  Marked as helpful

  0