NFT preview card component

Community feedback

• 

  Andreas Remdt•950

  @andreasremdt

  Posted over 2 years ago

  Hey @agnar-nomad,

  Nice job on the challenge! It looks so close to the design, great work! ::before and ::after can be tricky to understand, keep going and these things will become easier. :-)

  Some ideas to improve your code:

  • Your title "Equilibrium #3429" uses an h3 element, but since it's the first (and only) heading on the page it makes more sense to use an h1 instead. That's also the only remark on your accessibility report. Page headings should always start at the highest (h1) and incrementally go down until the lowest (h6).
  • You don't need to use both pseudo-classes, you can achieve the same result with just one (either ::before or ::after, it doesn't matter).

  .nft-container::before {
    content: "";
    position: absolute; 
    inset: 0;
    background: var(--cyan) url(images/icon-view.svg) center no-repeat;
    opacity: 0;
  }
  

  The above code achieves the same but is quite simplified:

  • You can combine background-color, background-image, background-position, etc. into a single shorthand property called background.
  • inset: 0 is a shorthand for top: 0; left: 0; bottom: 0; right: 0. It just saves time and lines of code :-)
  • Also, by using inset: 0, you don't need width and height as the element will stretch all the way.
  • content should be an empty string, since you set the background image via the background property.

  Additionally, I wouldn't use an ul element for the Ethereum price, since semantically it doesn't make the most sense. A simple div or p should do the job as well and is semantically more correct.

  I hope my feedback helped you, let me know if you have any questions :-)

  01

  Marked as helpful