Order summary component

Community feedback

• 

  Simone•170

  @Giulo25Posted almost 3 years ago

  Hi Fabricio, good job on this challenge! I think it is not necessary to set a width, in fact decreasing the viewport the image shrinks incorrectly

  You can remove the width: 1440px from the body and center the background image with background-position: top center

  You could also put padding on the body so in the mobile version there is space between the component and the edge of the device

  Marked as helpful

  0
• 

  Fabricio Rivera•90

  @FabricioRivera2021Posted almost 3 years ago

  Hi Thank both of you for the feedback, i really appreciate it. I made the changes to the css file to see if it fix the issues with the touching of the card on the mobile version. And change the background, the property max-width was really helpfull thanks. I think is better now, its not a 100% match though.

  0
• 

  Aakash Verma•6,710

  @skyv26Posted almost 3 years ago

  Hi! Fabricio, As I can see @Giulo25 have already gave feedback on your work but there are some other issues that @Giulo25 haven't told you.

  1. Your background image is not covering the full width of your desktop. So I went through your code and with testing below code solve width issue.

  body {
      font-family: "Red Hat Display";
      max-width: 1440px;
      height: 100vh; 
      background-image: url(./images/pattern-background-desktop.svg);
      background-repeat: no-repeat;
      background-color: hsl(225, 100%, 94%);
      background-size: contain;
      display: flex;
      justify-content: center;
      align-items: center;
  }
  

  I added the background-size: contain; so that it cover the full viewport width.

  See I change width: 1440px; to max-width: 1440px; because with only width property you are telling DOM that browser width should be 1440px and it also creates problem, like overflowing of your layout. In overflow your design will scrollable both horizontally and vertically. so use max-width, it tells the DOM that whatever the width of the screen is but it can't be greater than 1440px. I think you get it / understand it.

  2. I saw in mobile response, below 400px screen-width, your card is touching viewport left and right, both sides. Either add some padding/margin OR reduce the width of card and then use margin: auto; property to align your card.

  Overall Good, but need improvements. I hope you understand and soon will fix your issues.

  Best Wishes

  0