Frontend-Mentor-NFT-Preview-Card-Component

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, awesome work on this one. The layout in general looks great I think.

  MikevPeeren already pointed out a great tip, just going to add some other suggestions as well on the site:

  • It would be great to have a base styling of this:

  html {
    box-sizing: border-box;
    font-size: 100%;
  }
  
  *,
  *::before,
  *::after {
    box-sizing: inherit
  }
  

  This way, handling an element specially its size will be easier because of the box-sizing

  • Remember that whenever you create a site, always have a main tag that will wrap the main-content of the site. For this one, you can use main on the .card selector since that is the main-component. Then on the .attribution use footer tag on it so that it will be its own landmark.
  • Don't use position: absolute a very large element. If you inspect your layout, hover on either html or body tag, you will notice that it has no height since its element is being absolute. Since you are just using this to center the layout, it would be much better to do it this way. But first, remove these stylings on the .card selector:

  position
  top
  left
  transform
  

  Also remove the position on the .attribution as well. Then on the body tag, you can add these stylings:

  align-items: center;
  display: flex;
  flex-direction: column;
  justify-content: center;
  min-height: 100vh;
  

  This way, even if you zoom out or when a user have wide-screen, content will be centered properly. Though by using this one, I noticed that the overlay for the image is out of place, try to use position: relative on the parent container of that overlay so that it will be contained properly.

  • When using img tag, you don't need to add words that relates to "graphic" such as "image" and others, since img is already an image so no need to describe it as one.
  • Also, since you are treating the image as an interactive component, make sure that there is an interactive element present beside or nesting it. It could be a button if the component for example, clicking it would preview the nft image or use a tag if clicking the image will navigate user in another page to see the full nft information?
  • Also for the overlay img, it 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.
  • Since you are not using h1 as the nft name and an h1 should be present on a page, for this one, the h1 would be a screen-reader only heading. Meaning it will be hidden visually but present for screen-reader users. On this, the h1 would have like sr-only class and the text-content should describe what is the main-content is all about. The h1 could be placed as the first text inside the main.Have a look at this snippet of mine about screen-reader only h1 usage You can use something like a frontendmentor nft collection as the text-content of the h1.
  • Change the nft name to h2. When using a heading tag, make sure you are not skipping a level. If you use h3 then make sure that h1, h2 are all present before it.
  • Lose the icon word for the ethereum and for the clock icon, it is only decorative so hide it using the method I mentioned above.
  • Person's image should be using the person's name as the alt value like alt="Jules Wyvern". Components like this where a person's information is presented, make sure to use their name as the person's img alt value.
  • The person's name as well is being treated as an interactive component. Wrap the person's name in a a tag since it looks like a link to see the nft creator.

  Aside from those, great job again on this one.

  Marked as helpful

  1

  ---

  

  Pranav•50

  @Pranav1326Posted almost 3 years ago

  @pikapikamart

  First of all, Thank You so much for your guidance. I really appreciate that.

  I will try to make those changes that you mentioned and make it perfect.

  1
• 

  MikevPeeren•2,100

  @MikevPeerenPosted almost 3 years ago

  Hey 👋

  Good job finishing this one, try to keep in mind however when to use alt texts, for example for the icons it isn't needed but for the image a more descriptive text should be used.

  Marked as helpful

  1

  ---

  

  Pranav•50

  @Pranav1326Posted almost 3 years ago

  @MikevPeeren

  Thank You so much for your guidance. I really appreciate that.

  0