Frontend Mentor QR Code Card

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted almost 3 years ago

  Hi

  There’s quite a few styling issues here that need ironing out I think. First though, please indent your html consistently. It’s hard to read and a common source of bugs in bigger projects

  In html

  • remove b tag from heading, control font weight in css

  In css

  • size-adjust value is invalid and not needed. Not sure what you are trying to do there
  • font size must always be in a responsive unit, preferably rem, never px
  • remove max-height: 25%;. That shouldn’t be there. Again, not sure what you’re trying to do with that
  • remove the side margins from the cards inner elements, and width on the image. Let the image be display block and max-width 100% (very common reset for all images). Then give the card some padding. That’s what padding is for
  • this is very narrow on my mobile. I think that’s due to a width in % somewhere. Setting widths in % is often a bad practice because you can lose control of the sizes depending on screen size or size of parents. Instead I recommend keep the max-width on the card but using a little margin on it (or padding on a parent) to stop content hitting screen edges

  I hope this helps

  Marked as helpful

  1

  ---

  

  Anthony W.•10

  @yourpaldiggyPosted almost 3 years ago

  @grace-snow

  Thank you so much for all this insight. Since I've been writing JS for so long now, HTML has become an afterthought. I always make sure to indent properly, and consistently in JS. I just threw this together, which is no excuse for the final product being uploaded that way. I'll use this as a learning experience to make sure I do multiple passes often to make sure things are more orderly.

  I'm currently implementing your suggestions, and it's brought a few 'aha' moments(I need to study the difference between rem, px, em) since I've been out of CSS for just as long.

  Thanks again!

  1
• 

  Naveen Gumaste•10,480

  @NaveenGumastePosted almost 3 years ago

  Hay ! Good Job

  -> Add the font weight to the heading

  -> decrease the width of the card

  -> Add font size and weight to the paragraph too

  Keep up the good work!

  Marked as helpful

  1
• 

  Jordan Phillips•425

  @JordanPhillips-hubPosted almost 3 years ago

  Hey Anthony try using a <main> tag instead of a div on your bg this will give your page a main landmark as you have none in your HTML, also try using a h1 instead of an h4 and just adjust the font size to what you want your first header should be an h1 for accessibility reasons..

  Marked as helpful

  1

  ---

  

  Anthony W.•10

  @yourpaldiggyPosted almost 3 years ago

  @JordanPhillips-hub Thanks for the heads up! I’ll change those asap.

  0