NFT Preview Card Component Challenge

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, great work on this one and also, congrats on your first challenge here at FEM! Hope that you found doing the challenge exciting and learned something from it. At the moment, the layout could use some adjusting on it so that it will match the design and also, if you go to at mobile view, at around 370px, the layout just squished itself. If you are going to refactor it, then that would be really nice.

  Here are some suggestions for the site:

  • Don't use width: 100vw since this will only add a horizontal scrollbar at the bottom, since this value does not account the vertical scrollbar's width.
  • 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.
  • It would be great to have this markup:

  <main /> # contains the main-content
  <footer /> # contains the attribution
  

  This way, all content of your page will be inside their own respective landmark element. So instead of two div as the direct child of you body tag, it will be main and footer.

  • To make the layout centered, on your body tag, you could set these stylings on it:

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

  Normally, you will find yourself using those stylings a lot when centering contents as you go along.

  • You are missing the :hover state for the img. On your solution page of this challenge, you can go to the challenge hub and to the solutions so that you could see other people's submission on this challenge. You can always take note on how they approached a specific challenge.
  • You can use the nft name as the nft img alt value since it make sense to do so. It could be something like alt="nft Equilibrium #3429". Although the alt doesn't really describe the nft itself, but hey, nft is to be seen.
  • For now, you could use h1 for the nft name since a single h1 is needed on a page. The :hover state on it is missing as well. But on the nft name, your markup should be something like:

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

  You will need to use a tag because whenever you create an interactive component, interactive element must be used as well.

  • On this one, the eth-icon and the clock-icon serves only as decoration on the site. Decorative images are images that doesn't really contribute on the overall content of the site since they are only decoration. 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.
  • You could use the person's name as the person's image alt value since it makes sense to do so since their name is already present.
  • The person's name should be interactive as well. So inside the p tag, add an a tag to wrap the person's name.
  • Lastly, on your .central1 instead of using % for the width property, use rem so that it will be consistent for user. You can use % but only if you are properly controlling the size of an element.

  Aside from those, great job again on this one and looking forwards on your other challenges here at FEM^^

  Marked as helpful

  0

  ---

  

  Sprint DV•50

  @SprintStarDVPosted almost 3 years ago

  @martpika I'll try to come back to this challenge as soon as possible so that i can follow your suggestions to upgrade my design. Thank you so much for this detailed reply this is greatly appreciated!

  1
• 

  P

  David Turner•4,150

  @brodiewebdtPosted almost 3 years ago

  The design only needs to be about 350px. You don't need to resize it for every resolution.

  Hope this helps.

  Marked as helpful

  0

  ---

  

  Sprint DV•50

  @SprintStarDVPosted almost 3 years ago

  @brodiewebdt thank you!

  0