@Miculino
Posted
Congrats on completing the challenge, @jakubinhoo
Your solution looks really well done! And the responsive design works as expected. Good job!
Looking at the comparison screenshot, it looks like the background color is a bit different, but that's not an issue ;)
I also had a look at your Github code repo. Here are a few suggestions I have for you:
- You don't need this
width: auto; height: auto; margin: 0 auto;
The height is auto by default and you already set the width to 800px. Also, you don't need the margin property in that case because you've already centered the container using flex display
-
You can rewrite this line
grid-template-columns: 1fr 1fr 1fr;
using repeat function like thisgrid-template-columns: repeat(3, 1fr);
and if you want to have a single column layout for mobile screen you can do thisgrid-template-columns: repeat(3, 1fr);
-
Consider adding
min-height: 100vh
on body element so you know the container will be vertically centered no matter the resolution
Marked as helpful
@jakubinhoo
Posted
@Remus432 thanks a lot for your insights, I'll definitely pay attention to those details! :)