Design comparison
Solution retrospective
I had a few minor problems aling the way the way thats why my css is so messy lol plus i was too lazy to clean it up :D. im open to criticss as always :D.
Community feedback
- @correlucasPosted about 2 years ago
👾Hello @Chiemeka2006, Congratulations on completing this challenge!
Great code and great solution! I’ve few suggestions for you that you can consider adding to your code:
- Add the correct color for the background, that in this case is
background-color: #F2F2F2
- The html structure entirely with
div blocks
but these div doesn't any semantic meaning, for this reason is better you use a better html markup improving your code, for example for each vehicle card you use<article>
instead of the<div>
. - The icon doesn’t have an important role when you think about semantics and the html structure. So you can add
aria-hidden=“true”
to avoid it being found and read in the accessibility mode/screen readers. These are only decorative items. - Use
max-width: 100%
for the cards in the mobile version to allow the cards grow 100% of the width considering the paddings and avoid to have a lateral gap (limited by a fixed width). - To make your CSS code easier to work you can create a
single class
to manage the content that is mostly the same for the 3 cards (paddings, colors, margins and etc) and another class to manage the characteristics that are different (colors and icon), this way you'll have more control over then and if you need to change something you modify only one class.
✌️ I hope this helps you and happy coding!
0@Chiemeka2006Posted about 2 years ago@correlucas thanks for the feedback ill work on alll this stuff on my next project. <3 :D
0 - Add the correct color for the background, that in this case is
- @AdrianoEscarabotePosted about 2 years ago
Hi Chiemeka Okongwu, how are you?
I really liked the result of your project, but I have some tips that I think you will enjoy:
To align some content in the center of the screen, always prefer to use
display: flex;
it will make the layout more responsive!Example:
body { margin: 0; padding: 0; display: flex; align-items: center; justify-content: center; min-height: 100vh; }
The rest is great!
I hope it helps... 👍
0@Chiemeka2006Posted about 2 years ago@AdrianoEscarabote i already used display flex i was just having problems with the height cause the height wasnt covering the whole screen so align-content center wasnt working properly but i figured it out thanks fir the help :D
0 - @SrHatcherPosted about 2 years ago
hello Chiemeka!
To fix the problem with the height of the body i would add min-heigth: 100vh so that the body will have a height you could work with and then i would remove the padding top you have. Using height:100% doesn't work because the body at first doesn't know the total height of your screen (or even the total height of your page), and because of that the body height is set by the elements that are inside of it, unless you set a height for the body like i'm telling you (min-height:100vh).
By the way, i didn't know that using height: 100vh wasn't advisable, i would like to know why D:
You're doing a great work. keep going :)
0
Please log in to post a comment
Log in with GitHubJoin 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