Responsive NFT preview card component Using HTML5 and CSS3

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

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

  Some suggestions on the site 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.
  • But doing that makes the layout not centered anymore. So add an extra :

  align-items: center;
  display: flex;
  

  On the body tag.

  • Since you are treating the image as an interactive, since there is a hover with something like a preview state right, it would be great to use button in here. It would wrap the whole img so that user can properly focus on it and could toggle the preview if they want with just using keyboard.
  • Sine the Equilibrium text is an interactive element, supposing it would be a page-link where user can view the full information about the nft? What is nft :>> It would be great to use :

  <h1>
    <a href="/images/nft">Equilibrium </a>  # or just use href="#" for now
  </h1>
  

  • But to be honest, I wouldn't use h1 on the nft name in here, the h1 would be a screen-reader only heading inside the main. Since by looking at this component, on a real site, there would be numbers of different nft? Images in here so using the h1 as a general text would be lot better and using just h2 on the nft? Title.
  • The 0.041ETH should be inside a p tag. Remember that you don't wrap a text directly with a div element, always use a more suitable tag for texts.
  • The clock image is just a decoration on the site. 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.
  • Only use descriptive alt on images that are meaningful and adds content to the site otherwise hide the image for screen-reader users.
  • Again, use p tag on the 3-days-left.
  • Person's image should be using the person's name as the alt attribute since it is already visible, `alt="Jules Wyvern".
  • The person's name as well is treated as interactive component so use a interactive element on it like a a tag if this would like navigate a user on a different site.

  Aside from those, great job again on this one.

  Marked as helpful

  1

  ---

  

  P

  Michael Johnson•100

  @michaeljohnson-mjPosted about 3 years ago

  @martpika Thanks you so much. You just enlightened me with a lot of stuff which I didn't know. I will definitely try to improvise myself with the feedback you provided.

  Your detailed review also taught me how I can review other's code and what one needs to cover during a review. Thanks again.😄

  1