HTML, CSS, Flexbox, Media Query

Community feedback

• 

  Miroslav•180

  @miroslavdurinPosted over 2 years ago

  Hello Shamir, congratulations on finishing this project, it looks great :)

  First of all I think that page looks better if you put media query on 650px, I would do that first. Problem with the image is that in a mobile view, you are still using image from a desktop view. You can use <picture> tag to switch between different images, based on a screen size. I would suggest that you write in you HTML, instead of div.sub-container-1:

  <div class="sub-container-1">
        <picture>
          <source media="(max-width:650px)" srcset="images/image-product-mobile.jpg">
          <img src="images/image-product-desktop.jpg" srcset="images/image-product-desktop.jpg" alt="parfume">
        </picture>
      </div>
  

  If you don't like this way, you can also set image as a .sub-container-1 background-image, then you can change background inside of your media query.

  I would also suggest that you don't set image width and height in absolute units, you can use % instead of px, especially since you've already put fixed width and height on .sub-container-1. This is what I mean:

  .sub-container-1 {
    width: 300px;
    height: 470px;
  }
  
  .sub-container-1 img {
    width: 100%;
    height: 100%;
    display: block;
  }
  

  I would also remove all of border-radius properties, there is no reason to put them on 3 elements, you can put them on main container, like this:

  .flex-container {
    display: flex;
    border-radius: 10px;
    overflow: hidden;
  }
  

  I hope I managed to help you just a little bit. Good luck and happy coding :)

  0