NTF Preview Card Component

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @Yaser47 ,

  I have some suggestions regarding your solution:

  • To tackle the accessibility issues, You can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech).

  • There should be two landmark components as children of the body element - a main (which will be the NFT card ) and a footer (which will be the attribution).<Footer> should not be in the <main >. HTML5 landmark elements are used to improve navigation .

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

  • 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 . add hover effect on them .

  • The avatar's alt shouldn't be empty , you can use Jules Wyvern for it.

  • Read more how to write an alt text

  • 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 <ul> to wrap class="info" and in each <li> there would be <img> and <p>`` .

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

  • General point : remember a css reset on every project. That will do things like set the images to display block and make all browsers display elements the same.

  • Nesting css selectors not a good thing . Really important to keep css specificity as low/flat as possible. The best way to do that is single class selectors.

  • The hover effect on the image is not working you might chech the classe name

  
  .parent  **.image** {
  }
  .parent **.main-image** {
  }
  .parent  **.image:before** {
  }
  .parent **.image:hover:before** {
  }
  
  

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

  Marked as helpful

  0

  ---

  

  Yaser47•260

  @Yaser47Posted over 2 years ago

  @PhoenixDev22 Thank You

  1
• 

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  Hi Yaser, I took some time to look at your solution and you did a great job!

  Also I have some tips for improving your code:

  • add main tag and wrap the card for Accessibility
  • add descriptive text in the alt attribute of the image
  • You can add the effect :hover creating a div that appears on hover. I used tailwind but you can still see and understand which css properties you can use to do the same. Look here -> my solution
  • try to add a little transition on the element with hover effect

  Overall you did well :)

  Hope this help and happy coding!

  Marked as helpful

  0