Product preview

Community feedback

• 

  Angel M•370

  @csmurillo

  Posted over 2 years ago

  Hello Ashwin, Your frontend application looks similar to the solution provided. However, their is a lot of room for improvement. I made a list of things you may want to add next time around:

  1. Wrap all the main content of a particular page with the semantic tag <main></main> Main Tag

  2. Use Flexbox when you want to center things. I look through your code and saw that you used:

  .wrapper {
      ...
      position: absolute;
      top: 50%;
      left: 50%;
      -ms-transform: translate(-50%, -50%);
      transform: translate(-50%, -50%);
  }
  

  you should instead use:

  .wrapper{
      ...
      display:flex;
      justify-content:center;
      align-items:center;
  }
  

  this is the common method to center content, it is easier and more simpler than what you coded.

  3. Stop using margin to position content. Instead use flex box because when you create frontend web application you always want to conserve the web page content flow. Margin should only be used to separate content from one div to the other which is different to using margin to position where content should be in a webpage. When to Use Margin.)

  Flexbox works as follows, when you use display:flex; on a parent container, its children flow side by side. That said:

  In your code you can add flex on the .container:

  .container{
      display:flex;
  }
  

  Also if you want to fix your project use this styles.css: This is your styles.css I just removed all the margin and added flex box where needed and fixed the size of the webpage using width:100%; height:100%; inside html,body. However I still left for you to add padding and fix the content where needed.

  @import url('https://fonts.googleapis.com/css2?family=Montserrat:wght@500;700&display=swap');
  @import url('https://fonts.googleapis.com/css2?family=Fraunces:opsz,wght@9..144,700&display=swap');
  /* font-family: 'Fraunces', serif; */
  * {
      margin: 0;
      padding: 0;
  }
  
  html,body{
      background-image: url("vecteezy_banner-template-background-geometri-for-background_12921076.jpg");
      background-size: cover;
      width:100%; height: 100%;
      font-family: 'Montserrat', sans-serif;
  }
  
  .wrapper {
      background-color: #ffefd8 ;
      /* width: 90vw; */
      /* height: 80vh; */
      width: 100%; height: 100%;
      display:flex;
      align-items: center;
      justify-content: center;
      box-shadow: 0 4px 8px 0 rgba(0, 0, 0, 0.2), 0 6px 20px 0 rgba(0, 0, 0, 0.19);
  }
  
  .container {
      width: 550px;
      height: 450px;
      background-color: #FFFFFF;
      border-radius: 10px; 
      display: flex;
  }
  
  img {
     width: 275px;
     height: 450px;
     border-top-left-radius: 10px;
     border-bottom-left-radius: 10px;
  
  }
  
  p {
      font-family:'CustomFont', Montserrat;
      color: #696969;
      font-size: 14px;
      font-weight: 500;
  }
  
  h1 {
      font-family: 'CustomFont', Montserrat;
      font-weight: 500, 700;
  }
  
  #para {
      color: #696969;
      
  }
  
  #price {
      color: #18A558;
      margin-top: 30px;
      font-family: 'Montserrat';
  }
  
  #old-price {
  
  }
  
  button {
      width: 215px;
      height: 40px;
      background-color: #18A558;
      border-radius: 8px;
      border: 1px solid #18A558;
      cursor: pointer;
      color: #FFFFFF;
      padding: auto;
  }
  
  svg {
      padding: auto;
      margin-right: 12px;
  }
  

  More Stuff that may be useful:

  1. Mobile First Approach Mobile First Articles

  2. Alt In Images: I would suggest reading this article Image Alt

  3. PX vs REM: I would suggest reading this Pixels Vs Rem

  IMPORTED FONT PS: The way you imported the font is wrong. Always remember to select the font weight and add font-family: 'Montserrat', sans-serif; or font-family: 'Fraunces', serif; to the div/ or whatever tag that requires it.

  If you have any questions just reply to this message.

  02

  Marked as helpful