@Islandstone89
Posted
Hello. I'm not quite sure if you should use <article> or just <div> for the cards..otherwise, the HTML looks good, well done.
Here are some tips on the CSS:
-
Performance-wise, it's better to link fonts in the
<head>of the HTML then using@import. -
It's good practice to include an even bigger CSS Reset at the top.
-
I see you commented out
display: grid;
place-content: center;
but that's a perfectly fine way to center the component horizontally and vertically. You could also use Flexbox:
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
A few more lines, but works the same. In both cases, you can use gap to set the spacing between the <main> and the <footer>. I would recommend doing one of these 2 methods. Then, you can remove margin: 5rem auto on main.
-
Media query should be in rem, and it should target a smaller screen size, maybe around 50rem or so.
-
Remove
flex-direction: row- this is the default. -
max-widthshould not be in %. About 80rem should be fine.
Marked as helpful
@kwokkw
Posted
Hello @Islandstone89,
Thanks for your advice again. After reading your comment and making the changes. I have couple questions if you do not mind.
- Why it's better to link fonts in the
<head>of the HTML then using@import. Does website load faster in this way? - I do not understand why media query should be in rem. Also, what do you mean by targeting a smaller screen size and 50rem?
Thank you very much for you advise.
@Islandstone89
Posted
Yes, I think so. I'm far from an expert on these things, but I know Grace Snow keeps reminding people about it, and she has a lot of knowledge. So, I'm kinda passing it on from her.
Regarding media queries, again, I'm passing on information I have picked up from people with a lot of knowledge. However, I did read an explanation just a few days ago, in this excellent article by Josh Comeau.
So, your media query is targeting 1440px, equal to 90 rem. You probably chose that breakpoint because it's the size referred to in the style guide. But this is a common misunderstanding. It only means that it should look like that on 1440px, not that the breakpoint should be 1440px. In this case, the cards start to get too "stretchy" around 800px(50rem), so that's when the layout should change. Makes sense? :)
Marked as helpful
