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

@manjiriphatak

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


Hello everyone! Please go through my code and let me know of any improvements that I can work on. it took me 4 hours to get this done...is it extremely slow? I would like to know if

  1. My css is well structured?
  2. media queries are good or not

Community feedback

Rafal 1,395

@grizhlieCodes

Posted

Hiya,

Don't worry about 'slow' and 'fast', especially if you're starting out. You will improve with speed the more you code, and if you initially take your time than just do that and learn as much as possible in each project.

As for feedback:

I would structure this a bit differently. You can create 2 divs (sections) within the container for the image and the text.

<div class="container">
     <div class="img-container">
     </div>
     <div class="text-container">
     </div>
</div>

This already allows you for a bit more control imo.

You can then follow up by adding padding to the text-container. I saw that you pushed down the .container by adding padding-bottom to your cancel-order-section. This isn't necessarily the best practice.

Padding should be used by a parent to style the content inside of itself. Margin is used to interact with other elements around it.

So if we use padding to add space to the text-container it would make more sense.

.text-container {
    padding: 32px;
}

Everything in text container will now be positioned a bit better.

We can then take care of the other margins you have. Instead of applying them to every other/single element we can do a slightly more systemic approach.

.text-container {
    display: flex;
    flex-flow: column nowrap;
    justify-content: start;
    align-items: center;
    gap: 16px;
}

Now our elements have at least 16px of gap between them. You don't need to set this for every single element, instead just set it for the container (why I suggested the approach initially tbh).

Whilst looking at the design the next thing I spotted is that the first element, heading, has 16px of gap, but the other elements actually have 24px between them. We can fix this easily:

.text-container > *:not(:nth-child(1)) {
    margin-bottom: 8px;
}

So now an additional of 8px will be added to margin-bottom to every element except heading.

I have more tips etc but I decided to make a video on it instead of just typing away here. I'll message you when I've uploaded the video. It should be relatively helpful and should explain the above concepts a bit more easily, especially if you could see all the code.

I have videos on other projects that I cover 'best practices' in. Feel free to check them out as they are frontend mentor projects :)

1

@manjiriphatak

Posted

@grizhlieCodes Thankyou so much for all these tips and for explaining this is detail. if it not too much trouble can you please explain tis section again?

Whilst looking at the design the next thing I spotted is that the first element, heading, has 16px of gap, but the other elements actually have 24px between them. We can fix this easily:

.text-container > *:not(:nth-child(1)) { margin-bottom: 8px; } So now an additional of 8px will be added to margin-bottom to every element except heading.

1
Rafal 1,395

@grizhlieCodes

Posted

@manjiriphatak Sure thing. Firstly, do you understand how display: flex and how gap work? Just generally speaking, you don't have to be an expert in either.

0
Rafal 1,395

@grizhlieCodes

Posted

@manjiriphatak Ended up recording a video where I do this project and also covered the bit you enquired about, have a look if you're interested.

Link.

The actual answer to your question can be found at : 18:29 - Styling text container.

0

@manjiriphatak

Posted

@grizhlieCodes Thanks for explaining that. I didn't know about gap and now I will research it a bit more and work from there.I need to learn more about flex and grid and will do just that. Good effort on the video 👍🏼(just a suggestion - the bg music is a bit too loud sometimes and hard to catch you speaking) :) Thanks again.

1
Rafal 1,395

@grizhlieCodes

Posted

@manjiriphatak Daaaarn, must have forgot to balance it out. Thanks for the feedback 😁

0
Tarık 330

@developertarik

Posted

I think you should reduce your height a little and other than that it's really nice

0

@manjiriphatak

Posted

@developertarik Yes. Thankyou. Working on that now 👍🏼

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