Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

NFT preview card component

#styled-components
Agnar 220

@agnar-nomad

Desktop design screenshot for the NFT preview card component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


All feedback appreciated.

I think I finally understand ::after and ::before and I have learnt to do image overlays in this project. It was fairly easy after struggling with the previous tasks.

Community feedback

P

@andreasremdt

Posted

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 :-)

Marked as helpful

0

Agnar 220

@agnar-nomad

Posted

@andreasremdt Hello friend,

no questions, you have made very good points.

Thank you for taking the time to write this and helping me reduce my code. Much appreciated. I will include changes shortly.

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord