NFT Preview Card Component Main

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @zachidev ,

  Congatulation on completing this challenge. I have some suggestions regarding your solution:

  HTML

  • Since there's a :hover state on the image and means it's interactive, So there should be an interactive element around them. When you create a component thet could be interacted with a user , always remember to include interactive elments like (button, textarea,input, ..) for this imagine what would happen when you click on the image, there are two possible ways:

  1: If clicking the image would show a popup where the user can see the full NFT, here you use <button>.

  2: If clicking the image would navigate the user to another page to see the NFT, here use <a>.

  • The link wrapping the equilibrium image should either have Sr-only text, an aria-label or alt text that says where that link takes you.

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

  • look up a bit more about how and when to write alt text on images. Learn the differences with decorative/meaningless images vs important content

  • For this class="between" , you may use <ul> instead of only <div> and <span> as they are not sementics, and in each <li> there would <img> and <p> (I don't recommend to wrap a meaningfull content using span and div only.)

  • If you wish to draw a horizontal line, you should do so using appropriate CSS. Remove the <hr />, you can use border-top: to the avatar's part .liner.You can read more here. And also it is problematic for people who navigate with the aid of screen reading technology.YOU CAN READ HERE IN MDN

  • The avatar's alt should not be image avatar it's meaningless. You can use the creator's name Jules Wyvern. Read more how to write an alt text

  • To use more semantic tags , you may use <figure> and <figcaption> for the avatar's part.

  CSS:

  There are so many ways to add the hover effect on the image , The one I would use, using pseudo-elements to change the teal bg color to a hsla. Then opacity can be changed from 0 to 1 on the pseudo element on hover as there is no reason to have the extra clutter in the html.

  • Changing base html size. This has huge accessibility implications for those of us with different font size or zoom requirements.

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

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

  Marked as helpful

  2
• 

  Ahmed Bayoumi•6,740

  @Bayoumi-devPosted over 2 years ago

  Hey! Congratulations on completing this challenge... You have accessibility issues that need to fix.

  • Document should have one main landmark, Contain the component with <main>.

  <main>
     <div class="container">
        //...
     </div>
  </main>
  

  • Page should contain a level-one heading, Change h2 to h1 You should always have one h1 per page of the document... in this challenge, you will use h1 just to avoid the accessibility issue that appears in the challenge report... but don't use h1 on small components <h1> should represent the main heading for the whole page, and for the best practice use only one <h1> per page.
  • All page content should be contained by landmarks, Contain the attribution with <footer>.

  <footer>
     <div class="attribution">
        //...
     </div>
  </footer>
  

  Hope this is helpful to you... Keep coding👍

  Marked as helpful

  1