NFT Preview Card Solution by Vee

Community feedback

• 

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  Hi Vee, great work on this challenge! 😉

  Here are a few tips for improve your code:

  • to make it look as close to the design as possible decrease padding to card class
  • not use <br> to break the line of text
  • use min-height: 100vh instead of height to .container class
  • instead of using px or % use relative units of measurement like rem -> read here

  Overall you did well 😁 Hope this help!

  1

  ---

  

  Vee•240

  @JunasVeePosted over 2 years ago

  @denielden Thanks for the feedback, Daniel.

  The reason why I use % in every padding and margin is to make it adaptive.

  If I was about to make a media query and drastically change a particular height or width of an element, I wouldn't need to set new padding or margin value in the same media query, but at the same time, I can have the same output-looking in different viewports.

  Nevertheless, I'll definitely implement your useful tips. Why wouldn't I listen to a generous coder with 20K points 😂👍

  1
• 

  riverCodes•300

  @riverCodesPosted over 2 years ago

  The image shows up for me, it looks really nice! I was facing a similar issue and here's what helped me:

  • Check if there is a forward slash '/' at the start of your 'src' attribute. If there is, remove it.
  • Clear your browser history.

  I recently completed this challenge as well and looking at your code, i think we had a similar approach. Though your code does look cleaner, mine is all over the place :P As an extra suggestion, you can wrap your attributions in a footer tag, which helps accessibility.

  1

  ---

  

  Vee•240

  @JunasVeePosted over 2 years ago

  @riverCodes Thanks for the feedback!

  Somehow I genuinely forgot about the footer tag existence while making small projects like this😂.

  Thanks for reminding me and giving me the solutions for the image visibility issue.

  1
• 

  Darko•980

  @dtomicicPosted over 2 years ago

  Congrats on finishing the project, the picture shows up for me (if you're talking why there isn't a picture in the design comparison that sometimes messes up due to paths and stuff like that but no need to worry that's why there is a live preview of the site) as for the other stuff everything looks pretty nice, the card and everything else is responsive and looks good on small and big screens.

  Only thing I would recommend is to avoid adding <br> tags when you're trying to break up text (here) because when you get to bigger screens and the text naturally goes into a new line it will break up on the <br> tag and wont look nice after (this is how it looks on bigger screens). What I would recommend is to set the width of the text container or font-size until the text breaks into new line naturally and then you'll get a nice look and can easily adjust it.

  Overall good job 👍

  1

  ---

  

  Vee•240

  @JunasVeePosted over 2 years ago

  @dtomicic I'm glad to know that the pic shows up on others' devices, I'll also remove the <br> tag ASAP to make the text looks nice! Thanks for the feedback.

  0