Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

NFT preview card

2001Kaye 320

@jhan117

Desktop design screenshot for the NFT preview card component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Community feedback

PhoenixDev22 16,950

@PhoenixDev22

Posted

Greeting @jhan117,

I have some suggestions regarding your solution:

  • It's better not to use <div>for meaningful elements. <p class="">Our Equilibrium collection promotes balance and calm.</p>

  • For any decorative images, each img tag should have empty alt="" 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 avatar image . It's meaningless , you can set Jules Wyvern to it. And for the equilibrium image should be descriptive.

  • 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.

  • You can use an unordered list<ul> to wrap class="card-info" and in each <li>, there would be <img > and<p> . Don't use span's for meaningful content .

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

  • you can use<figure >or <figcaption> for user-info.

  • min-height: 100vh; to the body 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.

  • If you set a page width, choose 100% over 100vw to avoid surprise horizontal scrollbars.

  • height: 543px;It's rarely ever a good practice to set heights on elements

  • width: 327px;an explicit width is not a good way . Remove the width from the main component and change it to max width instead. That will let it shrink a little when it needs to.

  • 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.

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

Marked as helpful

1

2001Kaye 320

@jhan117

Posted

@PhoenixDev22 I was touched by your great feedback. Thank you :)

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join 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