@geomydas
Posted
Hi, it seems to me that this is your very first solution on Frontend Mentor. I wish you the best and have a fun journey coding! Anyways, here's my feedback
1. HTML
- Your HTML is almost perfect already but there is some minor problems. I reccomend removing the
<section>tags since it is just 1 card after all. You would typically use the section tag to divide unrelated content but this isn't the case here.
2. CSS
-
Use a CSS reset. What it does is that it makes your CSS look the same in most devices and it makes your life much more easier since it removes the default so you don't have to wrestle with it. You just copy and paste it in your code, and voila! I reccomend using Andy Bell's or Josh Comeau's CSS reset since they are the most popular and most used ones. You should also use them in all of your projects
-
Don't set fixed widths! Use a combination of max-width in a fixed unit such as rem or px and a width in a percentage unit, but most of the time the width is set to 100%. Using max-width instead of width makes your content shrink instead of being fixed which may cause overflow issues. Check line 28
-
You can omit the max-height in your body property. Even though a height is set already, it does noting because its exactly equal to height property. Also, replace
height:withmin-height:instead in your CSS, you can use this tip aswell for centering divs in the whole viewport height. If you use height, the body will not have a scroll and will hide content overflowing causing it to be unresponsive.
If this helps you, don't forget to mark it as helpful!
Marked as helpful
@Antonio-Riccelli
Posted
Hi @geomydas,
Thank you for your feedback, it's very helpful.
- What would I use instead of
<section>in this case? Would<div>be appropriate? - The tip about combining max-width and width: 100% was extremely useful - thank you. It basically solved all layout issues I was having.
- I did implement a reset but it was a very basic one. I have now added Josh Comeau's one.
@geomydas
Posted
@Antonio-Riccelli Divs dont carry meaning at all and it would appropriate for this case. Divs are just containers for styling most of the time. For the CSS reset, remove the basic one and only use Josh Comeau's. Thats pretty much it
