Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Order summary component

@FabricioRivera2021

Desktop design screenshot for the Order summary component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


Hi i got a question about this one, and the nft proyecto too... i dont know if it was ok to put the width for the desktop device, at first i was only gonna do the widht: 100vw; aproach like i did on the last one.

what is most correct, put the width of the devices or just let it adjust with the viewport units?.

Community feedback

Simone 170

@Giulo25

Posted

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

@FabricioRivera2021

Posted

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,690

@skyv26

Posted

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.

  1. 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

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord