Nft preview card with Flex

Community feedback

• 

  Andreas Remdt•950

  @andreasremdt

  Posted over 2 years ago

  Hey @Toxgem,

  Nice work on this challenge! It looks quite close to the design and has no HTML or accessibility issues, which is great!

  Here are a couple of possible improvements I can give you:

  • Using the below code, you can easily align your card in the horizontal and vertical center of your page. min-height is used to make the page as big as your browser's window, otherwise the body will only grow as much as the content inside requires it to.

  body {
    display: grid;
    place-items: center;
    min-height: 100vh;
  }
  

  • The way you solved the image hover effect is quite creative and interesting, but there are better ways. Try looking into CSS pseudo-elements, like ::before and ::after. They allow you to style HTML elements that are not really in your markup, but are still there for you to use. It's a bit tricky to explain in text, so you can check out my solution to see how I solved it.
  • You can try to reduce the amount of HTML used. For example, your h1 contains a span, which in return has a CSS class for color. You can remove the span and apply the class directly on the h1. The same applies to the below paragraph, which is inside a div without any styling. You can safely remove the div, as it doesn't do anything.
  • Using custom margins for spacing is the way to go, I don't see anything wrong with how you did it :-)
  • I would also go with rem in most cases. However, I use other units depending on what I am trying to style, e.g. px for border widths. Kevin Powell has a great video explaining when to use which unit, maybe this clears things up for you. https://www.youtube.com/watch?v=N5wpD9Ov_To
  • For the Ethereum and clock icon, you used img elements. While this works, it's not optimal in terms of semantic meaning. img elements should be used whenever you want to show an image that's relevant for the content, like a picture of a car on a car sharing site. These two graphics, however, are more for styling purposes and to make the layout look good, so it's better to use CSS background-image for that and remove them from the HTML. If you want to keep using img elements, make sure to apply aria-hidden="true" to them so that screen readers don't think that this is an important piece of content. Also, you can remove the alt text by setting it to alt="". Alt texts should only be present on images that have a meaning to the content.

  Hope that my tips made things clearer for you, if you have any questions feel free to drop them in the comments :-)

  11

  Marked as helpful