Frontend Mentor | NFT-PREVIEW-CARD coding challenge solution

@PhoenixDev22

Posted over 2 years ago

Hello @Adeyemi-Abiade ,

I have some suggestions regarding your solution:

Wrap the body content in main landmark.
You can remove the extra container class="card_bg_container".
Remove the margin:auto from the class="card". no need for it as you are using the flexbox.
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 .
Page should contain a level-one heading . In this challenge , you can use <h1> to wrap Equilibrium #3429. For future use , Heading levels should only increase by one.
No need for <br> , you can use margin and padding for spacing.
For any decorative images, each img tag should have empty alt="" (as you did )and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images in( icon-view, icon-ethereum, icon-clock ).
The avatar 's alt shouldn't be The Design's Creator. It's meaningless , you can set Jules Wyvern to it.
Read more how to write an alt text
the icon-view doesn't really need to be in the html, you could do it with css. If you want it to stay in html it needs to be aria-hidden or role presentation with empty alt.
the link should be wrapping the main image and either have Sr-only text, an aria-label or alt text that says where that link takes you.
I would leave the equilibrium main img in html and use pseudo-elements to change the teal bg color to a hsla. Then opacity can be changed from 0 to 1 on the pseudo on hover.
You can use an unordered list<ul> to wrap class="eth-time-bar" and in each <li>, there would be <img > and<p> .


        <ul  class="wallet-stats">
            <li class="wallet-info">
               <img src='./images/icon-ethereum.svg' alt="" aria-hidden="true"/>
               <p class="coin--text">0.041 ETH</p>
            </li>
            <li class="wallet-info">
               <img src='./images/icon-clock.svg'/ alt="" aria-hidden="true">
               <p class="time--text">3 days left</p>
            </li>
        </ul>

No need for <hr> , you can use border-top: to the class="author-info"
width: 350px; an explicit width is not a good way . Remove the width from the card .That will let it shrink a little when it needs to.
To center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body> not the class="card-bg-container"like this:

body{
display:flex;
align-items: center;
justify-content: center;
width: 100%;
min-height: 100vh;/*using vh (viewport height) units to allow the body to set a minimum height value based upon the full height of the viewport.This also allows the body to to grow taller if the content outgrows the visible page.*/


.img_cont{
    max-width: 100%;/** no need to set an explicit width/
    min-height: 300px;
    /* margin: 0 auto; */ remove the margin 
:
:
}

You should use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.
Never use px for font size.
Last , the repo of the NFT solution is not accessible . You might have a look at my solution , to see how I did the hover effect on the image , it might my helps.

Overall, your solution is good, Hopefully this feedback helps.

NFT-PREVIEW-CARD

Design comparison

Solution retrospective

Community feedback

Please log in to post a comment

Join our Discord community