@Islandstone89
Posted
HTML:
-
Remove the
<header>- it is used for content that would repeat on every page, like a navigation bar with a logo. The whole card should be inside the<main>. If you want to include the.attribution, that should be the<footer>. If so, its text must be wrapped in a<p>. -
I would wrap the date in a
<time>tag, like this:<time datetime="2023-12-21">21 Dec 2023<time>
CSS:
-
Performance-wise, it's better to link fonts in the
<head>of the HTML than using@import. -
It's good practice to include a CSS Reset at the top.
-
font-sizemust never be in px. This is bad for accessibility, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead. -
Change
heighttomin-height- this way, the content will not get cut off if it grows beneath the viewport. -
Remove
text-align: lefton thespan, as that is the default value. I would also removewidth: fit-contentand change fromdisplay: blocktodisplay: inline-block. -
The card should not have a fixed width. Instead, add a
max-widthin rem, to prevent it from growing too wide on larger screens. Around20remshould work OK. -
I would remove the media query altogether, as the design doesn't change. When you do use them, they should be in rem, not
px. It's common to do the mobile styles as the default, not the other way around. You wouldn't needwidth: 100%, andpadding: 1remshould be placed on thebody, and not inside a media query. With that set, you wouldn't need themargineither.
Marked as helpful
@eldon6219
Posted
@Islandstone89 improved the code and uploaded it thanks for your help appreciate it <3
@Islandstone89
Posted
@eldon6219 the pleasure is mine - well done!
As you're not using the --font-size variable, I would remove it. If you do use it, you must use rem.
Body doesn't need a width: 100%, as that is its default value.
Keep up the good work 🙂
