NFT Card

Community feedback

• 

  optimusprime202•1,200

  @optimusprime202Posted over 2 years ago

  Hey @jackiecodes2021, Wonderful work!

  0
• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hello, Jackie! 👋

  Congratulations on finishing this challenge! 🎉

  About the overlay image, I would recommend wrapping the img with an interactive element first. Then, you could use pseudo-elements and CSS background properties to create the overlay. For your reference, you could see my solution and read the README to get some general idea about it.

  Some feedback from me:

  • Accessibility
    • Every img tag should have alt="" attribute.
    • Use interactive elements (a) for any elements that have :hover or :active states.
    • After that, create a custom :focus-visible styling to any interactive elements (button, links, input, textarea). This will make the users can navigate this website using keyboard (by using Tab key) easily.
    • Alternative text for images should not contain any words that related to image (e.g. picture, photo, logo, icon, graphic, etc). It's already an image element so the screen reader will pronounce it as an image. 🙂
    • Always wrap text content with a meaningful element (p). Only use div and span for styling purposes.
    • Use the creator's name as the alternative text of the avatar.
    • Use CSS border property to create the line. hr element has a role as a separator. In this case, the content below the line should not be separated.
    • Use rem or sometimes em unit instead of px. Using px will not allow the users to control the size of the page based on their needs.
  • Styling
    • Don't limit the height of the card element. Let the content inside the card dictate the height of it.
    • Currently, the card element is not perfectly centered. To make the card perfectly in the middle of the page, you can make the body element as a flexbox container. After that, you could remove the margin from the main element.

  /**
   * 1. Make the card vertically center and
   *    allow the body element to grow if needed
   */
  body {
    display: flex;
    align-items: center;
    justify-content: center;
    min-height: 100vh; /* 1 */
  }
  

  • Best Practice (Recommended)
    • Always use classes to reference all the elements that you want to style. Using id is going to make your stylesheet have high specificity (hard to maintain).
    • Remove the commented code. If another developer (it can be you in the future) wants to improve this solution, he/she might get confused about whether or not the commented code should be used or deleted.
    • Also, if you are planning to add an attribution, you should put the attribution outside of the main element and wrap it with footer.

  That's it! I hope this helps! 😁

  0
• 

  Kevin Mullen•100

  @Kmullen444Posted over 2 years ago

  Image Hover Is what was recommended to me and it really helped.

  0