@Islandstone89
Posted
Hi, well done!
Here is some feedback - I hope it is helpful :)
HTML:
-
Every webpage needs a
<main>
that wraps all of the content, except for<header>
andfooter>
. This is vital for accessibility, as it helps screen readers identify a page's "main" section. Wrap the card in a<main>
. -
Replace the IDs with classes. IDs have a higher specificity than classes, so are harder to override. This article explains the best use cases for the
id
attribute. -
"Learning", "Published", and "Greg Hooper" are all
<p>
elements, as they don't introduce any new content. -
I would wrap the date in a
<time>
element:<p>Published <time datetime="2023-12-21">21 Dec 2023</time></p>
. -
You should never have more than ONE
<h1>
on a page. The<h1>
is reserved for the main heading on the page. Since "HTML & CSS" is a card heading, it would likely not be the main heading of the page. Hence, I would make it a<h2>
. -
The heading needs a link inside, as this is a blog card.
-
Avoid using "picture" or "image" in the alt text - screen readers announce images with "image", so having that in the alt text would make it redundant.
-
.attribution
should be a<footer>
, and you should use<p>
for the text inside.
CSS:
-
Including a CSS Reset at the top is good practice.
-
I like to add
1rem
ofpadding
on thebody
, to ensure the card doesn't touch the edges on small screens. -
I would move
font-family
tobody
, since it should apply to all elements. -
I don't quite know why, but I've heard from an experienced source that using
display: grid
andplace-content: center
for centering can cause overflow issues. An alternative approach is to use Flexbox:
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
min-height: 100svh;
-
Remove the fixed width on the card. It's OK to set fixed sizes on smaller elements, like the profile image in this instance. For layout purposes, it should be avoided, as we need our components to grow and shrink according to different screens.
-
We do want to limit the width of the card, so it doesn't get too wide on larger screens. Give the card a max-width of around 20rem to solve this issue.
-
font-size
must never be in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead. -
You can use the
outline
shorthand to setoutline-color
,outline-style
andoutline-width
:outline: 1px solid black;
. -
Not necessarily needed for this project, but I would get into the habit of creating Custom Properties for values that are reused, like
background-color
,color
,font-family
,font-weight
etc. This makes things easier, as you only need to update the value in one place.
Custom Properties are usually declared on :root
, meaning they are global, i.e. they can be used anywhere. For example, let's say you wanted to store your font as a Custom Property. Every Custom Property must start with --
followed by some text: in theory, that text could be anything, but it's best to name it something meaningful:
:root {
--ff-primary: "Figtree", sans-serif;
}
You would then set it on the body, using var(--variableName)
.
body {
font-family: var(--ff-primary);
}
Hope this makes sense, and good luck! :)
Marked as helpful
@ElkuchWaltz
Posted
Thank you for more helpful feedback, @Islandstone89! I've gone in and done my best to follow your advice for tweaking the code.