@pikapikamart
Posted
Hey, great work on this one. Both desktop and mobile layout is good, the responsiveness could be better but still you did great considering you only learn it about 2 weeks!
Some suggestions would be:
- Adding a
padding
to all the sides of thebody
tag would be great to prevent the layout from touching the sides of the screen when it is being scaled down. - On the hero image, on the
.img-area
selector, you don't really need to specify thewidth
andheight
of it, since theimg
inside it will give the container the size it needs. Then you could just add theheight: 100%
andwidth: 100%
to theimg
inside it so that it will scale when the browser get's resize and will not overlap the container. - Also, just a tip. Avoid using
height: 100vh
on a container, like thebody
tag. When you use this, thebody
tag will only have a fixed height and that is the height of your screen. So when the content of your website is much greater than the height of your screen, technically, yourbody
tag doesn't not captures them. Instead, a practice, usemin-height: 100vh
. This will use the full height of the screen, but it will expand if it needs to, like when the content is large in terms of height.
Other than that, you really did great in here.
Marked as helpful
@Elioskas
Posted
@pikamart Thanks a lot for very detailed review. I used the 100vh height on the body so that I could center everything more easily, but it didn't occur to me to use min-height. :)