NFT preview card

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  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

  @jhan117Posted over 2 years ago

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

  0