only HTML and CSS

Community feedback

• 

  David Miguel Andrés•270

  @damigandPosted 4 months ago

  The div container for the image is unnecesary. I'm assuming the following code is used to make sure the image is properly aligned and contained within its flexbox parent:

  CSS

  .img-container{
      width: 256px;
      height: 256px;
      margin-top: 10px;
      display: flex;
      flex-direction: column;
  }
  
  img{
      /* Image */
      border-radius: 20px;
  }
  

  HTML

  <div class="img-container">
       <img src="images/image-qr-code.png" alt="">
  </div>
  

  However, this code can simply be replaced by the following:

  CSS

  img {
      width: 100%; 
      max-width: 256px;
      margin-top: 10px;
  }
  

  HTML

  <img src="images/image-qr-code.png" alt="">
  

  By using width: 100%; we ensure that the image's width will be, at most, 100% of its parent, eliminating the need of a wrapper div. In addition, we can also ensure that the image remains as a square with the property aspect-ratio: 1 / 1;, but in this case, it's not necessary.

  I also suggest you fill the alt="" property on images. In this case, you could use alt="qr code for Frontend Mentor".

  Overall this looks very solid. On wider screen sizes, the background color doesn't fully extend both in height and width, leaving unwanted white areas. This can be fixed by adading width: 100%; and height: 100%; to both html and body elements. The padding around the QR code image is a bit too wide, but other than that, the project looks very, very similar.

  Good job!

  Marked as helpful

  0