Design comparison
Community feedback
- @Islandstone89Posted 11 months ago
Hi there. I'm afraid your solution needs quite a lot of improvement. Here are the things that need to be fixed.
HTML:
-
<link>
tags should be in the<head>
, not the<body>
. -
You need a
<main>
, this is important for accessibility. Replace<div class="main">
with<main class="card">
. -
The image must have alt text. This is essential for screen readers to understand the image. The alt text should be descriptive, and in this example, it also needs to say where it leads (frontendmentor.io).
-
Remove all of the divs, there is no need for any in this challenge.
-
"Improve your frontend skills" is a heading, not a paragraph. Make it a
<h1>
. -
Remove the
<br>
tag - all styling should be done in the CSS. -
Remove all of the circle divs.
CSS:
-
You should keep the CSS in an own file, often called
style.css
. -
It's good practice to include a CSS Reset at the top.
-
height
on thebody
should bemin-height
. -
Remove all
positioning
properties. -
On
.main
, removeheight
andfamily
, which is not a property. -
Remove all fixed widths and heights.
-
Font-size must never be in px. Use rem instead.
-
Put
text-align: center
on thebody
, and remove it elsewhere. -
There is no need for any media queries, as the design doesn't change.
-
Image should have
display: block
andmax-width: 100%
. Remove the margin. -
To create the space between the image and the edge of the card, use
padding
on the card itself. -
Remove
display: inline-block
andmargin: auto
on the card. -
Put this on the
body
to properly center the card horizontally and vertically:
display: flex; flex-direction: column; justify-content: center; align-items: center; min-height: 100vh;
Marked as helpful0 -
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