Design comparison
SolutionDesign
Community feedback
- @grace-snowPosted about 2 months ago
I can see some issues here in the live version and code. (It would be great if you updated code so people didn't need to go digging through different branches to find it too!)
- don't add empty divs in html if you can avoid it. Background images are bad for performance too so it would be much better if you just used an img element for the main image.
- the link on the blog name should be inside the heading not wrapping it.
- there shouldn't be any other link in this card, especially not wrapping all of the author content. You should never have links wrapping multiple meaningful elements anyway as it really messes with the accessible name of the link.
- get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use.
- the container should not have a height. Nor should the card. Never limit the height of anything that contains text. Instead, the container or body should have min-height 100svh so it can grow taller than that when it needs to. The card doesn't need height or min height at all. Let the browser do it's job and decide what height is needed based on the content inside.
- the card also shouldn't have a width. Give it a max width in rem instead. This let's it shrink narrower when it needs to.
- to center the component on the screen use flex column properties along with that min-height 100svh on a wrapping element.
- there's no benefit at all to making the card into a grid with place content center. Only do that when you need to. Make sure you are only adding properties that you really need.
- to make the whole card clickable you can use a pseudo element on the heading link. There is no need for a separate link.
- don't style on IDs. Use classes.
- don't set a height or width on the category/tag. That should come naturally from the content and it's padding.
- font size should never be in px. Use rem. https://fedmentor.dev/posts/font-size-px/
- the hover should be on the link not heading. Once the link is inside of it this should be easier.
- I recommend against using nth of type selectors like this. You should be ensuring code is readable. You want to be able to come back to this later and read the css and immediately understand what it's doing and why. These selectors make it confusing.
- once you've updated the html you should be able to remove the media query as its not needed in this challenge.
Marked as helpful1@Fikerte-TPosted about 2 months ago@grace-snow thank you for your time to review my code and for the constructive feedback. I'm learning a lot
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