@Alex-Archer-I
Posted
Hi!
Congrats with your first challenge =)
Here is a few suggestions about css and stuff.
- It's not a good idea to use 62.5% trick. You can read about it here (and it's a good article about css units at all).
- To make your card more responsive it's better to use
max-widthinsteadwidth. But you can combine them is you doesn't want your element take all the width on the mobile screens.
main {
width: 95%;
max-width: 75rem;
}
- As for the
heightit's better not to specify it at all. In real projects content could change time to time, so it's more convenient when height of the container depend on the content (but not forbody- there you can usemin-heightto prevent overflow). - And there is no need to wrap image into the
div- you can style it directly. More of that - it's quite a simple layout so you can just make it like this
main
img
h1
p
main
Better not to create extra tags if it isn't necessary.
Overall it's a neat work, good start, keep doing =)
@Alex-Archer-I I somewhat agree with the use of the 62.5% font-size trick and the idea of placing the image within a <div>. However, I don't fully agree with your suggested layout approach.
When I initially worked on this project, I followed the Figma design precisely, ensuring every detail was aligned with the provided specifications.
If you closely examine the Figma design, you'll notice that there is specific padding applied to the text component on the left and right - view screenshot
Here's the HTML structure after your suggestion (removing div wrap):
<main>
<div class="card">
<img src="./assets/images/image-qr-code.png" alt="qr code" loading="lazy"/>
<div class="card-content">
<h3>Improve your front-end skills by building projects</h3>
<p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
</div>
</div>
</main>
This approach allows for flexibility, making it easy to add multiple cards with different images and text content as needed.
I typically think of layouts from a component-based perspective, which is why I prefer this method.
For reference, there is a useful example of a card layout at w3school
Thank you for your recommendations and for taking the time to review my work. I appreciate your feedback!
@Alex-Archer-I
Posted
@RobertsPeirags
Oh, yeah, I can see now why you need a div here (I would still apply padding for the both text blocks with shared class, but both approached are valid =)).
It's just I'm a used to think about that the browser parse tags into the nested JS objects and we could reduce the load by reducing the tags. But guess it'll be noticeable only with like 100 tags or more =)
Anyway, glad that something from my suggestions came useful =)
