NFT Preview Card Component

Community feedback

• 

  P

  Jeff Guleserian•480

  @jguleserianPosted almost 2 years ago

  @JasonSolarz

  Jason,

  I liked the look of your solution for the NFT Preview Card Challenge. Specifically:

  • your html structure is clean and simple, and thus easy to follow
  • likewise, your CSS is minimalist and easy see the relation with your HTML
  • the card itself looks identical to the original

  On the other hand, I was curious as to why you did not code the overlay on the top image, the hover effects on the title and creator's name, or the @media breakpoints. Perhaps this is only your first draft and you have more coming. Having access to your README.md file would have been helpful in this respect. I'll assume you have more coming, but if not, if you left them out because you are still working on those skills, let me know and I can suggest some resources. That said, I do have a few suggestions that may help in terms of best practices:

  • Screen readers will want to see an <h1> title in the body element so they can announce the subject/topic of the page. Most users don't need this, of course, so you will want to hide the <h1> from site. You can accomplish this by adding a class, like "sc-only," and then in the CSS, something like:

  .sr-only { position: absolute; top: auto; height: 1px; width: 1px; overflow: hidden; }

  • because pixels may cause issues with accessibility, it seems to be common practice to use em or rem instead. There is still some discussion out there about it, but there are legitimate reasons to favor these more versatile measurements. If you are interested, you might take a look at EM vs REM vs PX – Why you shouldn't “just use pixels”
  • I think the challenges assume that when you use components, as opposed to entire sites, that the component be centered vertically and not just horizontally. To do this, I recommend that in your main container, the one that will house your component, you use a grid and justify and align the content, something like: *container* { width: 100 vw; min-height: 100vh; display: grid; justify-content: center; align-content: center; }

  I hope these suggestions help in some way. I'm sure you have resources that will help you with the breakpoints, overlays, and hover effects, but if not, or if I can be any help at all with those matters, please respond and let me know.

  Again, impressive job on recreating original model. Happy coding!

  Jeff

  Marked as helpful

  1

  ---

  

  Jason Solarz•60

  @JasonSolarzPosted almost 2 years ago

  @jguleserian

  Hi Jeff,

  Thanks for taking the time to review my code and providing ample feedback.

  I've went back and updated the missing features that you pointed out. The only thing I left alone was the @ media breakpoints. The width of the container is 350px, which falls below the mobile design of 375px. For best practices, should this be added anyway or would this be needed if I was using versatile measurements instead of static pixels.

  I will review the blog post you shared about measurements.

  Take care,

  Jason

  0