NFT Preview Card with React

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, really nice work on this one and congrats on your second project here at FEM!. The overall layout of the site I think looks great. Just an issue about it is that, when you zoom out, the bluish background of the nft container just fills up the whole width of the screen, you might want to check that one out.

  But for some other suggestions, here are some:

  • For this one, it would be really nice to use main and footer. Change the .App to using main and the .attribution to use footer tag so that both will be contained inside of a landmark element.
  • The header on this one is used wrong sorry. A secondary header or just header as sectioning should be used not by itself only. For example, you could use article where there is a header but not on this one. You can just replace it with a div and it would be fine.
  • Since there is a :hover state on the image, meaning that it is interactive, then there should be an interactive element present as well and not only the img tag. When you create a component that could be interacted with user, always remember to include interactive elements like button or a tag. For this one, since when hovering on it, there appears a preview-icon, imagine what will happen if the user clicks the image. If clicking the image would just show a pop-up where the user can see the full nft, then go with button, but if by clicking it redirects the user in another page to see the nft, then go with a tag. Both could be wrapping the img but I would prefer to just place it beside it. I haven't tackled this one out yet so I can't give a reference on this one:>
  • For the nft image, you can use the nft name as the alt since it is already present like alt="nft Equilibrium #3429". I know that the value is somewhat still ambiguous because it doesn't really represent the nft looks. But since nft is to be seen, I would prefer to still add the alt.
  • The img for the preview-icon is only decorative. 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 you are using svg instead of img tag.
  • For this one, you could use the nft name as the h1 since a single h1 is needed for a page of a site. Wrap the a tag within the h1:

  <h1>
    <a href=""></a>
  </h1>
  

  • When using img tag, you don't need to add words that relates to "graphic" such as "icon" and others, since img is already an image so no need to describe it as one.
  • The person's image could be using the person's name as the alt value since it is already present like alt="Jules Wyvern".
  • Same for the person's name, it is being treated as an interactive one so maybe wrapping the person's name within an a tag would be really nice.
  • Lastly, just addressing the issue when zooming out:>

  Again, great job on this one.

  Marked as helpful

  2

  ---

  

  Rio Cantre•9,690

  @RioCantrePosted almost 3 years ago

  @pikapikamart Got it! I appreciate the feedback. I'll take notes on all of this.

  1
• 

  Dev Rathore•2,600

  @GitHub-dev12345Posted almost 3 years ago

  Hey Dude can you tell me how to make your simple portfolio website & which framework used for our portfolio, tell me and which site to deploy your portfolio

  0

  ---

  

  Rio Cantre•9,690

  @RioCantrePosted almost 3 years ago

  @GitHub-dev12345 Hi! I used React and deployed it with Netlify.

  0

  

  Dev Rathore•2,600

  @GitHub-dev12345Posted almost 3 years ago

  @RioCantre can you share this porfolio code

  0

  

  Rio Cantre•9,690

  @RioCantrePosted almost 3 years ago

  @GitHub-dev12345 Credit me afterwards.

  0