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 with Flex

#accessibility
Marcos 40

@Toxgem

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


Id like some reviews and best practices for this project

  1. spacing between elements i did with some inline-flex and customs margins but i am sure there are better ways

  2. the sizing of the cards, i've been using rem since it takes root element but i want to know how you would do it.

  3. if there is any visible adjusment that you would make

Thanks in advance for anyone that takes the time to review it

Community feedback

P

@andreasremdt

Posted

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

Marked as helpful

1

Marcos 40

@Toxgem

Posted

@andreasremdt Thanks you for taking the time and reviewing it, i'll try to implement it all

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