NFT Card using HTML and CSS only

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  Hi there! 👋

  Congratulations on finishing this challenge! 👏

  I have some feedback on this solution:

  • Accessibility
    • Well done on using landmarks correctly! 👍
    • Use interactive elements (a) for any elements that have :hover or :active states.
    • Always wrap text content with a meaningful element (p).
    • For decorative SVGs, add aria-hidden="true" attribute to the svg.

  <svg aria-hidden="true">
  </svg>
  

  • Alternative text for images should not contain any words that related to image (e.g. picture, photo, logo, icon, graphic, avatar, etc). It's already an image element so the screen reader will pronounce it as an image. 🙂
  • The alternative text for images should not be hyphenated.
  • Use the creator's name as the alternative text for the avatar.
  • 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
    • I would recommend using flexbox to align the icon and never use inline styling. It has a high specificity and can cause a serious issue on bigger projects.
    • I would recommend making the body element as a flexbox container to position the card in the middle of the page, both vertically and horizontally. Then remove all the margin properties from the container 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)
    • Remove the xmlns attribute from the inline SVG. It doesn't need it by the browser.

  That's it! I hope this helps! 😁

  Marked as helpful

  2

  ---

  

  ckullo•60

  @ckulloPosted almost 3 years ago

  @vanzasetia As always, you give me a kick-ass review... I really appreciate and love it. Some of your suggestions have already been applied, I'll update the finished update later.

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  @ckullo The link is broken though... (https://ckullo.github.io/csb-7njvi8/).

  0

  

  ckullo•60

  @ckulloPosted almost 3 years ago

  @vanzasetia Sorry for that... Now it's fixed.

  0
• Account deleted

  Hi there 👋

  Congratulate on finishing your project 🎉. You did a great job 💡

  I give some suggestions to help you take your project design to the next level 📈😉

  • Add box-shadow to the card
  • Change the h1 font-size to 24px
  • Add cursor: pointer to the h1 and .creator-name elements

  Happy coding ☕

  Maqsud

  Marked as helpful

  2

  ---

  

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  @maqsudtolipov I would not recommend adding a cursor: pointer to the h1. It's an interactive element and also the users would expect something happen when they click the Equilibrium #3429 text.

  As a user, I would expect it will navigate me to a page about the Equilibrium. So, I would recommend doing this instead.

  <h1>
    <a href="/">Equilibrium #3429</a>
  </h1>
  

  Marked as helpful

  1

  Account deleted

  @vanzasetia Thanks Vanza, compared to me you look professional 👍, can you give some tips to improve code reviewing tips

  0

  

  ckullo•60

  @ckulloPosted almost 3 years ago

  @maqsudtolipov I've updated the source with your suggestion on:

  1. Add box-shadow
  2. Change font size for h1 to 24px

  0
• 

  optimusprime202•1,200

  @optimusprime202Posted almost 3 years ago

  Hey @ckullo, You’re on the right track now.

  0