Hello Ali!
First I want to say that you did a great job providing this solution! Please don't get discouraged by my feedback; we're all here to learn and to improve our frontend skills. I hope my review will be helpful in guiding you toward writing better code and achieving your goals.
Overall:
At first glance, your solution looks very close to the design. However, as I started reviewing the code, I noticed a few areas where thing could be improved. I'll go over each point to help you refine your solution.
quick tip: you're using vscode, so i suggest you install prettier plugin if you haven't already, it auto-formats your code, making it more clear, or prettier for a reader, and you don't have to even think about it
HTML:
- Use of
main tag: This is a small point at this stage, but it's a good, semantic practice to wrap your main content in a <main> tag. For example:
<main>
<div class="box">
*content here*
</div>
</main>
- Making the link clickable: The "HTML & CSS foundations" text is meant to be a clickable link, but it's not currently set up as one. You can wrap it in an
<a> tag, like this:
<a href="#">
<h3 class="subtitle">HTML & CSS foundations</h3>
</a>
CSS:
- CSS Reset: At the beggining of your CSS, you attempted a simple reset. If you want to only reset margins, that's fine, but a more complete reset often includes properties
box-sizing and padding as well:
* {
box-sizing: border-box;
margin: 0;
padding: 0;
}
- Avoid hardcoding width and height: In your
.box class, you manually set the width and height properties. It's generally beter to let the content dictate the size. A good approach for responsiveness is setting the max-width property tho.
You shouldn't worry about this too much at this moment, as you'll learn it at the beginning of second learning path on frontend mentor, just be sure to check out every learning resource provided!
- Use
padding instead of margin-left: Instead of manually positioning the content inside the container using margin-left, you can apply padding to the .box class itself. Similarily, use the gap property with flexbox to manage spacing between elements inside the card.
- Improving the
.title class: Instead of setting a fixed width of 70px, you can use max-width: max-content to allow the title to grow with the content. Additionally, adding padding can help ensure the text doesn't feel cramped. Here's an updated version:
.title {
padding: 5px 15px;
max-width: max-content;
background-color: #f4d04e;
border-radius: 3px;
}
- Handling image in this card: To ensure the image inside
.box looks good and fits well, you can use the following:
img {
width: 100%;
border-radius: 8px;
}
This way, the image will automatically take up the whole width of the card, excluding the padding
- Simplifying margins: If you want to apply the same margin on all sides, you can simplify it by using:
margin: 11px;
And if you want to center an element horizontally, you can use margin: X auto, where X is the margin top and bottom.
- Hover, Active and Focus states: One thing missing from your code is the hover, active and focus states for interactive elements (like a link inside
<a> tag. ** But this right here would be a homework for you. I want you to learn about this and apply it to your card!**
Right here is a codepen with your code modified by me, it's not exact 1:1 to the design, you still might need to tinker a little with it, but it contains every change I talked about above: codepen link
Keep up the great work, and I hope these tips help you on your frontend journey!
