Design comparison
Solution retrospective
I used margin to center the image which I feel was improper let me know!
Community feedback
- @kimodev1990Posted 11 months ago
To center your image :
- you could add in your div class .container :
display: flex ; justify-content: center ; align-items: center ; padding: 1 rem ; as you wish
- Then It's better to insert & wrap image - here and in future designs - under div so you could have better responsive design, In HTML :
<div class= "image"> <img src="image location" alt=""> </div>
- Then in CSS ;
.image { width: 100% ; display: flex ; } .image>img{ width: 100% ; }
P.S : There are several ways to center the image, But this is my approach when I code designs like these.
- You could use clamp ( ) method in your coding for font-size, width, margins, padding, etc., So the designed sizes will change according to the viewport dimensions having a responsive design and will be suitable for any device layout.
Hope you find this Helpful.
Other than that, Really Nice work & keep Going on
Marked as helpful1 - @Islandstone89Posted 11 months ago
Hi, here is some feedback - hope it helps :)
HTML:
- To make the code clearer and more readable, it's convention to indent child elements. Here, we have a
<main>
, with a.container
child, which again has an image, a heading, and a paragraph. It should look like this:
<main> <div class="container"> <img> <h1> <p> </div> </main>
and the
.attribution
on its own line, on the same indentation as<main>
.-
The alt text should be descriptive, and in this example, it also needs to say where it leads (frontendmentor.io). Never write words like "image" or "photo" - screen readers will announce that automatically.
-
Headings should always be in order, so you never start with a
<h3>
. Change it into a<h1>
. -
Do not use
<br>
to force text onto a new line. The text should flow naturally, and any potential styling belongs in the CSS. -
.attribution
should be a<footer>
. -
Footer text needs to be wrapped in a
<p>
.
CSS:
-
It's good practice to include a CSS Reset at the top.
-
font-size
must never be in px. This is bad for accessibility, as it prevents the font size from scaling with the user's default setting in the browser, Use rem instead. -
body
should have amin-height
instead of aheight
- this way, you don't risk content being cut off if it grows beneath the viewport. -
Remove the fixed width in
px
, and the width in percentages. You rarely want to set either, as this easily creates issues with responsiveness. -
Add a
max-width
of around 20rem on the card, to prevent it from getting too wide on larger screens. -
Remove the
margin
on the image. To create the necessary space between the image and the edge of the card, usepadding
on all 4 sides of the card. -
Add
display: block
andmax-width: 100%
on the image - the max-width prevents it from overflowing its container. -
Both the
text-align
andfont-family
properties have the same values for all elements in this challenge. Hence, you should set both on thebody
, and remove it elsewhere.
Marked as helpful1 - To make the code clearer and more readable, it's convention to indent child elements. Here, we have a
- @rafacardoso17Posted 11 months ago
Congratulations on excellent work. I liked your project; it turned out a bit compact. But if your goal was to match the model, if you allow me to give a tip, I would suggest using tools like Figma to identify dimensions of borders, images, etc. It helps me a lot, and I think it would help you too. Once again, congratulations on the project.
1
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