@Islandstone89
Posted
Hi, well done submitting your solution. Here are some suggestions to make this even better.
HTML:
-
Every page should have a
<main>for the main content. I would change your.overall-containerto<main class="container">. -
Don't use
<br>to style the text. HTML is all about content, let the CSS do the presentation. -
.improveshould be a heading. -
For a challenge as simple as this one, you don't need to wrap the image in a div.
CSS:
- First, you need some basic CSS "resets" at the top of the stylesheet:
*,
*::before,
*::after {
box-sizing: border-box;
}
img {
display: block;
max-width: 100%;
}
-
On the
<body>, changeheighttomin-height. Also, addalign-items: centerto center the card vertically. -
Hence, you can remove the margin on the card - the card will be centered horizontally and vertically using Flexbox. Do also remove the fixed height and width, we generally don't want to set fixed dimensions. But add a
max-widthin rems on the card. -
Remove the margin from the image. To get the space around the image, use padding on the card itself.
-
You are using the wrong font. See the style guide for the link to the correct Google Font. Also, the font-family should be set on the
body. -
Since all text on the card should be center-aligned, it's better to set
text-align: centeronly once, on thebody. The text elements are children (or technically grandchildren) of the body, and so they will inherit its values.
Hope this is clear and helpful :)
Marked as helpful
@Mastermind390
Posted
@Islandstone89 Thanks for your suggestions and I really appreciate.
I will look into all what you pointed out and make necessary changes.
Thanks once again.
@Islandstone89
Posted
@Mastermind390 My pleasure :)
