Frontend Mentor | Musread's profile

All comments

@lamepicSubmitted 12 months ago

Result Summary Component

@MusreadPosted 12 months ago

Hello 👋

This is an excellent solution for this challenge. Congratulations for that !

My only suggestion will be on design appearence for the circle result. Indeed, if we look closely, the gradient color go from a violet color to transparent. I made a pull request on your GitHub project, so that you can see the suggestion. Otherwise, I have nothing more to say about your solution.

Great job 👍

Marked as helpful

Hasan Sertaç Yiğitoğlu•150

@hsyigitogluSubmitted over 1 year ago

I'm waiting feedbacks from you

Order summary card with html, css and bootstrap 5

Musread•130

@MusreadPosted over 1 year ago

Hi Hasan 🙂 !

I'm impressed with your solution ! This a great work !

I was looking at your code and I have some little suggestions that may improve your skills at coding great web interfaces.

# 1st suggestion

The first suggestion is about the image's borders in the top of the card. Indeed, you used the border-radius property like this :

.card{
    width: 400px;
    height: auto;
    border-radius: 25px;
}
picture {
    border-radius: 25px;
    margin-bottom: 10px;
}
picture img{
    width: 100%;
    border-top-left-radius: 25px;
    border-top-right-radius: 25px;
}

That is great, because this is exactly what we need to have shaped borders. But, as developers, it's important to think about the don't repeat yourself philosophy.

Maybe you've already heard this expression which makes somebody looks like a genius 😂, but it's very simple to understand.

In the code above, you used the border-radius property in three places. But if somehow you need to change the value to 20px, you will have to change it three times : in .card, picture and .picture img. This is easy here. But imagine for big projects in which you have multiples files and folders and you used a CSS property in 35 different places. The day you will need to change it, you will have to do it 35 times 😅.

So, I would suggest this code :

.card{
    width: 400px;
    height: auto;
    border-radius: 25px;
    overflow: hidden;
}
picture {
    margin-bottom: 10px;
}
picture img{
    width: 100%;
}

In HTML, when an element inside another one is bigger, it will 'overflow' it. Than means the part that is too big to fit in will be visible. But what's cool is that you can decide to hide that visible part, hence the declaration overflow: hidden. So with this solution, you will only need to change the borders for the .card. You can find more explanation in Mozilla documentation there.

# 2nd suggestion

The last suggestion I want to give is about the layout.

When you look closely at the design, the buttons and the rest of the content should have the same lenghts. But in your solution, the buttons are smaller than the rest of the content. Of course, you can chosse to put the same width for .card-btn and .card-text. But as I mentioned before : don't repeat yourself 😄.

What I suggest her is to put your .card-btn element inside the .card-text element. Then you can remove the place-items, so the element will automatically occupy the available width.

.card-content .card-btn{
    display: grid;
    /* So `.card-btn` will occupy all the width by default */
    /* place-items: center; */
}

All you need to do if you want to modify the width is to play with the padding of your .card-content element 🙂.

That's it for me !

I hope I dind't bother you with all that explanation 😂. I try my best to explain what I've learned, so feel free to ask questions if you want.

I made a pull-request on your GitHub repo so you can check out the modification I've made.

Bye 🖐 !

Marked as helpful