NFT preview card component using Flexbox and Grid

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Layout in general looks great!

  Some suggestions would be:

  • Avoid using height: 100vh on a large container like the body as this makes the element's height capped based on the viewport/screen's height. Instead use min-height: 100vh so that the element will expand if it needs to.
  • For this, it would be great to have a main tag that will nest the .card selector. This way your site will have a main-landmark.
  • Change the section into using div, supposing this would be a real site, there would be lots of .card and you don't want each of them to use section tag.
  • Since you are treating the image as an interactive component since there is the hover state and whatnots , consider having an interactive element like a button or a tag to be partner with the image. But I don't suggest wrapping the img with the element since the image inside it has a text, I would go for something like:

  <div class="nft__container">
    <a href="#">
      <span class="visually-hidden">View Equilibrium nft</span>
    </a>
    <img src="" alt=""> # nft image
  </div>
  

  The a tag's pseudo of ::after will be position: absolute and take full width and height of the .nft__container so that the whole nft will be clickable. The view-icon will be the background of the ::after as well. This might seem confusing hehe but I don't have a reference since I can't do this challenge(pro) but it goes something like that.

  • It would be great to have a screen-reader only h1 in this page. The h1 would be the first text of the supposed main tag and will describe what is the main-section is all about. Since this is like a nft previews, it could be:

  <main>
    <h1 class="visually-hidden">NFT preview collections</h1>
  ....
  ...
  </main>
  

  • I almost forgot, the view-icon is just decorative. Decorative images should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if the image is using svg.
  • When using img tag, you don't need to add words that relates to "graphic" such as "icon" and others, since img is already an image so no need to describe it as one.
  • Use only Ethereum as the alt of the eth icon.
  • This text 0.041 ETH should be inside a p tag. When wrapping up a text-content, make sure that it is inside a meaningful element like p tag or heading tag and not using like div, span to wrap the text.
  • The clock-icon is only decorative so hide it using the method above.
  • Person's image should be using the person's name as the alt value like alt="Jules Wyvern". Components like this where a person's information is presented, make sure to use their name as the person's img alt value.

  Aside from those, great job again on this one.

  Marked as helpful

  0

  ---

  

  Mohamad Adithya•290

  @mohamadadithyaPosted about 3 years ago

  @pikamart Okay, thank you for your advice/feedback. I will fix it later ;).

  1