Design comparison
Reports
All page content should be contained by landmarks
<div class="card shadow p-3 mb-5 border-0" style="width: 18rem;">Saumitra Paira’s questions for the community
This is my first submission on frontend mentor. I have tweaked mostly all the css property. Let me know if I have followed best practices properly. Any other feedback is greatly appreciated.
Community feedback
@kenreibman
Posted
Great job on your first submission!
I noticed that in your index.html you had
html body {
background-color: hsl(212, 45%, 89%);
}
you can just have in your style.css
body {
background-color: hsl(212, 45%, 89%);
}
I also noticed that you have a .body in your style.css as well which is looking for an element with the class="body" I think you were meaning to select the body element as a whole and center items within in. In that case, you should have also just done
body {
min-height: 100vh; <--- *NOTE I also changed this to min-height as it's better practice
width: auto;
display: flex;
align-items: center;
}
Marked as helpful
@saumitr
Posted
@kenreibman Thank you so much for your feedback.
The reason I was using html body in index.html was because I was not able to change background color of body in style.css. From online answers, I found that because I was loading bootstrap, it was setting to default color which was white, so I had to declare seperately.
But now that you explained that when declared as .body, it looks for class="body", and I should have instead declared body element as a whole in style.css, it makes a lot of sense and I tried it, it's working. I'm going to push the code changes shortly.
Thanks again for your valuable feedback.
Please log in to post a comment
Log in with GitHub
Join our Slack community
Join over 180,000 people taking the challenges, talking about their code, helping each other, and chatting about all things front-end!