@VenusY
Posted
Hey! I had a look at this project as you requested, and it's really good! Almost no issues at all from what I can see.
The site is responsive, and it resembles the design very closely.
The only thing I would suggest is that you should consider using padding
instead of margin
for the whitespace around the edges of the page.
This is because using margin makes it so that the site has scrolling at all times, even if the user's screen size is tall enough to fit all the content.
Using margin
causes this issue because it's not considered in the calculation of the element's width or height, even if the box-sizing
is set to border-box
.
border-box
only includes padding and border as part of it's width or height.
So this in combination with min-height: 100vh
means that the height of the webapge is always going to be 100vh + whatever the margin is, causing the scrolling issue.
I hope this explanation has been helpful, but if not, please feel free to ask me questions, and I'll try my best to clarify.
body {
margin: 2rem; ❌
padding: 2rem;
}
@media screen and (min-width: 800px) {
body {
margin: 4rem; ❌
padding: 4rem;
}
}
Other than that, you did a fantastic job at this challenge, and I can see that you're improving a lot! :)
Marked as helpful