NFT preview card component

Community feedback

• 

  P

  Robert McGovern•1,075

  @tarasisPosted about 2 years ago

  Hi congrats on finishing the challenge. Please when you submit it, link directly to the repo / folder with the code and not your Github profile. Its not clear looking at your profile that the repo contains your solution. For anyone coming along, solution is https://github.com/lumon2004/lumon2004/tree/Frontend-Mentor-Challenges/NFT%20preview%20card%20component

  I'm curious why you chose to use the Cabin font, rather the one indicated in the Style guide? Likewise the change of page background color.

  If you have a good reason that would be something perfect to add to the Readme.md (rename existing Readme.md to something else, then rename Readme-template.md to Readme.md. Then edit it to cover what youve done)

  I'd suggest not mixing styling between a style block and the html tags themselves. It makes someone reading your code have to jump around to understand what is going on.

  Try and use semantic html where approriate. For instance wrap the attribution in a footer tag. The container could be a main tag.

  Your alt tags are fairly descriptive which is good, although "NTF" is a touch vague.

  I don't think using h2-5 is the right approach for the text in the card as they aren't really headings. For instance the h3 for price and h5 for term/period of time.

  For the image, "Equilibrium #3429" and creator name you are styling them as if they are links. So make them links.

  Test at in between screen sizes using the responsive design mode in a browser. For instance your build is fine at 375px, but breaks horribly at about 425px until 1138px. This happens because your container is set to width: 20%; above 425px.

  Try and take a mobile first approach, and then adapt as needed for the desktop version. For instance use width: 80% as you have for mobile, but then have say max-width: 21.875rem on the container so it never gets bigger than that size. Then your solution will scale nicely from mobile to desktop (you'll need to tweak padding as well)

  Hope that helps, feel free to ask if I've been unclear, and best wishes for your next challenge.

  0

  ---

  

  Luca•90

  @lumon2004Posted about 2 years ago

  Hi @tarasis, your report about the repository link is so right, that's why now I'm gonna edit my past solutions and work on next ones. Then, I chose the Cabin font and not the one indicated in the Style guide just because I didn't read it, just like I already did in past challenges. So there's no a rational reason because I used it. I already know that mixing the CSS with the HTML it's not the most right thing to do but, as I already replied to another guy who commented one of my past challenges, I'm working in this way just for these challenges. Normally I do not work this way. I usually also work with semantic tags in HTML, as header, main and footer, but I'm not doing this in these challenges because it's recommended in one of the warnings in the hub of each challenge to use a <div id="container">. Surely it doesn't exempt me to use semantic tags, but I do not wanna complicate my life uselessly. Regarding to my alt tags, I couldn't do anything different from "NFT" except "Non-fungible token", which is the extension of the acronym. I used H2 and H5 even though they weren't headings just because in my mind those weren't paragraphs. I also know that my code looks horrible at those sizes, I have to get better at making responsive a webpage. Indeed, if you noticed, I used @media screen and (max-width: 415px) property because 414 is the width screen of any recent iPhones, which is the most used mobile phone. I want to thank you anyway for your comment and I'm gonna fix my errors, and a special thank to your last tip about the width, I'm gonna try it!

  0