Design comparison
Community feedback
- @EfelipaPosted 3 months ago
Nice classes and style. Avoid using unnecessary div's and add a border to the card. Other than that, great job!!
1 - @Islandstone89Posted 3 months ago
Great job!
I love how you have used Custom Properties, set a
max-width
on the card, avoided fixed widths or heights inpx
, and including the "lobotomized owl" selector :)Some tips:
HTML:
-
You don't need
aria-hidden: true
, since the alt text is empty. -
I would wrap the date in a
<time>
element:<p class="blog-meta">Published <time datetime="2023-12-21">21 Dec 2023</time></p>
. -
I would change the heading to a
<h2>
- a page should only have one<h1>
, reserved for the main heading. As this is a card heading, it would likely not be the main heading on a page with several components. -
As this is a blog card, the heading needs a link inside.
-
A better alt text for the profile image would be something like "Headshot of Gary Hooper.".
-
.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 the properties on
.blog-parent
to thebody
- this way you get to center the footer as well. -
You don't need to change the card's default
display: block
todisplay: inline-block
. -
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. -
I like to nest the
:hover
states inside the element selector, like this:
.blog-container { max-width: 23em; margin: 0 1.5rem; background: var(--white); padding: 1.5rem; border-radius: 1rem; box-shadow: 7px 7px 0 var(--dark-gray); &:hover { box-shadow: 10px 10px 0 var(--dark-gray); } }
- As mentioned, well done for using the
> * + *
selector - it is one of my favorite tools for creating vertical space between elements. However, I prefer to create a utility class that I can add anywhere in the markup:
.flow { --flow-space: 1rem; } .flow > * + * { margin-block-start: var(--flow-space); }
Then I would add it to the card:
<div class="blog-container flow">
The great thing is that if I wanted to adjust the top margin of one of the elements, I could override the default spacing. Let's say I wanted the heading to have a top margin that is only
.5rem
:.blog-title { --flow-space: .5rem; }
NB:
margin-block-start
is a logical property, which is recommended to use.Once again, well done, and keep up the good work! :)
0 -
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord