@Alex-Archer-I
Posted
Hi there!
Actually it is possible to review your css via the dev console =)
You did right thing with img by setting it's width to 100%, but as for card it's better to set max-width for it instead the hardcode one. Maybe it is no so important here (I doubt that screens less than 320px is a big thing, unless we talking about Apple Watch), but it is good practice in general.
Almost that could be said about height of the body tag. Use min-height instead. It'll prevent overflow if the content will be larger than the screen.
And yeah, you used correct semantic tags, but I can suggest you to reduce them a bit =) For such simple project you can get rid of the section and div, and use main tag as a container with img, h2 (or h1) and p.
Also you are already turn main tag in the flex, so you can use gap property to create spaces between inner elements. It's a bit easier than with margins (but can create only equal spaces).
So, congrats with your first challenge, you did great =)
Good luck and feel free to ask anything if need (I love parentheses too, by the by)
Marked as helpful
@vstm
Posted
@Alex-Archer-I Thank you for you thorough review, that's exactly what I was looking for, and I learned so much!
- Using
max-width/min-heightis a good point, I can see how those are in general better than the "absolute"width/heightproperties. - Regarding the semantic tags I agree, I just got rid of all the div's but yeah in retrospect I used semantic tags where the element was actually only used for display purposes and did not have
- Yes I should have used the
gapproperty, I think I was adding theflexclass after thespacing-*tailwindcss class and it looked fine so I did not change anything else. For any future tailwindcss uses I need to remember that if I add aflexorgridclass to any element I should also change thespacing-*classes togap-*classes.
Thank you again (and yeah parentheses are the best :D)
@Alex-Archer-I
Posted
@vstm
Really glad I could help =)
Also you can use max-width alongside with the width.
main {
width: 95%;
max-width: 75rem;
}
That way main element became 95% if it's parents (or screen size, depends of context) will shrink less than 45rem.
Sorry that I'm not using tailwind for examples, I'm not in deep touch with it (I prefer plain css + postcss plugins).
And just in case if you get e wrong, I don't meant that you should use less semantic tags. Just less tags in general =) Semantic tags in most cases are preferable. Again, you used them correct =)
