Design comparison
Solution retrospective
This was an easy one, i made this after taking a break from coding for a while, but it was fun!
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hello Abdelghafour,
Congratulation on completing this frontend mentor challenge.
I have some suggestions regarding your solution:
-
There should be two landmark components as children of the body element - a
main
(which will be the NFT card) and afooter
(which will be the attribution).<Footer>
should not be in the<main >
. HTML5 landmark elements are used to improve navigation . -
To tackle the accessibility issues: Page should contain a level-one heading . In this challenge , you can use
<h1>
withclass="sr-only"
(Hidden visually, but present for assistive tech). -
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
. -
For any decorative images, each img tag should have empty
alt=""
andaria-hidden="true"
attributes to make all web assistive technologies such as screen reader ignore those images in(icon-view, icon-ethereum, icon-clock
).
The link wrapping the equilibrium image should either have
Sr-only
text, anaria-label
oralt
text that says where that link takes you.-
The avatar's alt should not be avatar. It's meaningless, you can use the creator's name
Jules Wyvern
. Read more how to write an alt text -
Never use
<span>
or<div>
alone for meaningful content , you can use<p>
instead. -
To use more semantic tags , you can use <ul> to wrap
class="worth"
and in each<li>
there would be<img>
and<p>
.
You can use
<figure>
and<figcaption >
for the avatar's part.CSS
-
To center the component on the middle of the page , you can use the flexbox properties and
min-height: 100vh
for the<body>
(and you can remove the card's margin.) -
width: 353px;
an explicit width is not a good way . consider usingmax-width
to card instead and a little margin to the card .That will let it shrink a little when it needs to.
General points :
-
Really important to keep css specificity as low/flat as possible. The best way to do styling is single class selectors.
-
It's recommended to use
em
andrem
units .Bothem
andrem
are flexible, Usingpx
won't allow the user to control the font size based on their needs. -
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 helpful0@Abdelghafour122Posted over 2 years ago@PhoenixDev22 thank you so much for taking the time to write this, i really appreciate your feedback!
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