Design comparison
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hello David Pokrajac,
Congratulation on completing this frontend mentor challenge.
I have some suggestions regarding your solution:
-
<footer >
and<main>
are intended to be at the same level of hierarchy in the page (one level below<body>
). Nesting one inside the other would therefore not be recommended. -
I wouldn't use
<section>
in this challenge. Using<section>
tag for some parts of the card is not really a good choice . Section is not meant to be used anytime you feel tempted to use a div . section is for a bigger chunk of content often titled by<h2>
Read more about usage notes. -
Anything with a hover style in design means it's interactive. you need to add an interactive element
<a>
around the image ,Equilibrium #3429 and Jules Wyvern
-
The avatar's alt should not be A shot of a person.You can use the creator's name
Jules Wyvern
. Read more how to write an alt text
The link wrapping the equilibrium image should either have
Sr-only
text, anaria-label
oralt
text that says where that link takes you.-
The
<span>
HTML element is a generic inline container for phrasing content, which does not inherently represent anything. It can be used to group elements for styling purposes (using the class or id attributes), It should be used only when no other semantic element is appropriate. -
To use more semantic tags , you can use
<ul>
to wrapclass="info"
and in each<li>
there would be<img>
and<p>
. -
If you wish to draw a horizontal line, you should do so using appropriate CSS. Remove the
<hr />
, you can useborder-top:
to the avatar's part. -
For the same reason stated before , this part might look :
<p>Creation of <a href="#" class="">Jules Wyvern</a></p>
You can use
<figure>
and<figcaption >
for the avatar's part.CSS
-
The issue with setting only explicit widths in percentage , is you are losing control of the layout. (35vw or 20% is not the same in each device ).consider using
max-width
to card instead and a little margin to the card .That will let it shrink a little when it needs to. -
Consider using
min-height: 100vh;
instead of height: 100vh to the body allows the body to set a minimum height value based upon the full height of the viewport also allows the body to to grow taller if the content outgrows the visible page.- If you set a page width, choose
100%
over100vw
to avoid surprise horizontal scrollbars
- If you set a page width, choose
-
height: 65%
It's rarely ever a good practice to set heights on elements . Let the content inside the card element dictate the height of it.
General point:
- Remember a css reset on every project. That will do things like set the images to display block and make all browsers display elements the same.
Overall , your solution is good. Hopefully this feedback helps.
Marked as helpful2@DavidPokrajacPosted over 2 years ago@PhoenixDev22 thank you very much for your detailed insight.
0 -
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