@PhoenixDev22
Posted
Hello @NElnour ,
I have some suggestions regarding your solution:
-
Anything with a hover style in a design means it's interactive. you need to add an interactive element around the image
Equilibrium #3429. -
For any decorative images, each
imgtag should have empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images in (/icon-clock.svg", icon-ethereum.svg", /icon-view.svg) -
the
icon-viewdoesn't really need to be in the html and doen't need to be wrapped in an interactive element <a> , you could do it with css. If you want it to stay in html it needs to bearia-hidden:true ` orrole presentationwith empty alt -
The
alttext of the avatar shouldn't bealt="nft creator avatar", it should beJules Wyvern.Tips for writing 'good' alt text. -
it's invalid html to wrap a heading element in an anchor tag. Swap those around so the anchor is inside the heading
<h1><a href="#">Equilibrium #3429</a></h1> -
don't use
divfor meaningful content .you can use <p><p> Our Equilibrium collection promotes balance and calm.</p> -
the link should be wrapping the original image and either have
Sr-onlytext, anaria-labeloralttext that says where that link takes you. -
You can use an unordered list
<ul>to wrapclass="nft-status">and in each<li>, there would be<img >and<p>. -
don't use
spanfor meaningful content<p>Creation of <a href="#">Jules Wyvern</a></p> -
No need for
<hr/>, you can useborder-topto theclass="user-info".. -
To center the card on the middle of the page , you can use the flexbox properties and
min-height: 100vhfor the<body>like this:
body{
display:flex;
align-items: center;
justify-content: center;
width: 100%;
min-height: 100vh;
-
Never use
pxfor font-size. -
You should use
emandremunits .Bothemandremare flexible, Usingpxwon't allow the user to control the font size based on their needs. -
bg color to the image on the hover effect
Overall your solution is good , Hopefully this feedback helps.
Marked as helpful
