QR Code Component Challenge

Community feedback

• 

  T

  Chamu•13,130

  @ChamuMutezvaPosted over 1 year ago

  Hi Wodaz

  Much has been said already by Fer. Further to that , here is a few recommendations:

  1. this challenge can be done without media queries
  2. give the main element a class eg main and add the following styles. Take note that the main is the main container . Check the effect of these changes, I will remove some of the styles in the card class that have been replaced here

  .main {
  max-width: 20rem;
  margin: 2rem;
  }
  
  .card {
  background-color: hsl(0, 0%, 100%);
  /* margin: 0 auto; */
  /* max-width: 20rem; */
  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
  border-radius: 20px;
  }
  
  .top {
  /* width: 90%; */
  /* display: grid; */
  /* place-items: center; */
  padding: 1rem;
  }
  // just give the image a class
  .img {
  /* width: 310px; */
  /* height: 330px; */
  object-fit: cover;
  border-radius: 20px;
  /* margin-top: 5%; */
  display: block;
  max-width: 100%;
  }
  .bottom {
  /* width: 90%; */
  display: flex;
  flex-direction: column;
  row-gap: 25px;
  padding: 25px;
  text-align: center;
  font-family: 'Lucida Sans', sans-serif;
  padding-bottom: 13%; // use something like rems. maybe 1rem. 
  }
  

  The values used here are imaginary, you have to refer back to the design and replace to meet the designs. Remove all the media queries

  Marked as helpful

  0

  ---

  

  w0daz•220

  @w0dazPosted over 1 year ago

  @ChamuMutezva hey!

  thanks so much for this tip. it does reveal so much to me about how to properly style my elements without too much padding and margins... I'm still wrapping my mind around the ramifications. I cannot be grateful enough.

  0
• 

  Fer•3,970

  @fernandolapazPosted over 1 year ago

  Hi 👋, perhaps some of this may interest you:

  HTML / ACCESSIBILITY:

  • The QR code is a meaningful image and therefore should not be a background-image. It should have an alt text with a description in case the user can't see it.

  CSS:

  • It is better to use min-height: 100vh; for the body, as using height causes the page to be cut off in viewports with small height (such as mobile landscape orientation). Also, there's no need to set a width to 100%, as block elements are full width by default.

  • You might consider using relative units like rem or em since they are better for scalable layouts. Something simple to start with would be to convert to rem (1 rem equals the font size of the root element, 16px by default). Consider this suggestion especially for the font-size.

  I hope you find it useful, any questions do not hesitate 🙂

  Regards,

  Marked as helpful

  0

  ---

  

  w0daz•220

  @w0dazPosted over 1 year ago

  @fernandolapaz thanks for the feedback... i've effected the changes you proposed. can you take a look at the code again?

  0

  

  Fer•3,970

  @fernandolapazPosted over 1 year ago

  @w0daz

  Hi,

  The changes look good. 👍

  Keep in mind for the future that you usually don't need to wrap <img> elements with a <div>. Just remember to include this rule as part of your resets:

  picture,
  img,
  svg {
  display: block;
  max-width: 100%;
  }
  

  Greetings,

  Marked as helpful

  0

  

  w0daz•220

  @w0dazPosted over 1 year ago

  @fernandolapaz awesome. thanks. noted

  1