NFT preview card component

Community feedback

• 

  Kristina Radosavljevic•380

  @KristinaRadosavljevicPosted about 2 years ago

  Hi, Raymond! Very cool solution, it really looks great :)

  I noticed two things that you might want to consider:

  • I would add just a little bit of top and bottom padding on the body because the card sticks to the top/bottom edge of the screen if the viewport height is smaller than the height of the card.
  • A little disclaimer for this next one - I don't know all that much about Sass, I'm just starting to get into it myself - BUT I think there's a different way to define variables in Sass and it's much cleaner than the regular CSS way. You can define them as, for example, $cyan: hsl(178, 100%, 50%); and you don't have to do it inside the :root selector. And then when you use them in the code, you don't have to use the var() function, but just the name preceded by the $ symbol, as in background-color: $cyan;. Apparently, however, there's a difference in how CSS and Sass variables are compiled, so maybe read more on that (for example here) before you take my advice :D

  Other than that, really great job! :)

  Marked as helpful

  0

  ---

  

  Raymond Fajiculay•230

  @keyztrokeePosted about 2 years ago

  @KristinaRadosavljevic Thank you for your input, very much appreciated 👋

  1

  

  Raymond Fajiculay•230

  @keyztrokeePosted about 2 years ago

  @KristinaRadosavljevic Hi there👋, Can you please look at what I did if I understand your suggestion correctly? Your feedback is important to me. Kindly take a look if you have time. Thank you!

  0

  

  Kristina Radosavljevic•380

  @KristinaRadosavljevicPosted about 2 years ago

  @keyztrokee Yeah, now your Sass code looks much cleaner, I like that you've moved all the variables to a separate file, that's definitely a good practice to adopt when you move on to bigger projects :)

  1

  

  Raymond Fajiculay•230

  @keyztrokeePosted about 2 years ago

  @KristinaRadosavljevic Thank you so much I learned something new 😁

  1
• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  Hi, Raymond Fajiculay! 👋

  Congratulations on finishing this challenge! 🎉

  One suggestion I have is to keep the CSS specificity as low and flat as possible. There is no need to nest from the html element. I recommend using single-class selectors for styling whenever possible. Also, I recommend only nesting when you want to style pseudo-elements, pseudo-classes (:hover, :focus, etc), and @media queries.

  This was a mistake that I made when I was first using Sass. When I wanted to improve the site, the Sass files were hard to understand because of deep nesting. As a result, I had to rewrite the entire styling.

  Hope this helps!

  Marked as helpful

  0

  ---

  

  Raymond Fajiculay•230

  @keyztrokeePosted about 2 years ago

  @vanzasetia Thank you for your input, very much appreciated 👋

  1

  

  Raymond Fajiculay•230

  @keyztrokeePosted about 2 years ago

  @vanzasetia Hi there👋, Can you please look at what I did if I understand your suggestion correctly? Your feedback is important to me. Kindly take a look if you have time. Thank you!

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  @keyztrokee Great! Now, the stylesheet has low specificity. Also, the Sass code is much easier to understand. Good job! 👍

  1

  

  Raymond Fajiculay•230

  @keyztrokeePosted about 2 years ago

  @vanzasetia Thank you so much I learned something new 😁

  1