NFT preview card component

Community feedback

• 

  Richard Ogujawa 👨‍💻•170

  @RichardOgujawaPosted about 2 years ago

  Hi Guillherme!

  Hope you're keeping well:) First and foremost I really like how you've indented your HTML really makes it a lot easier to read, and I also like how you put headings in your HTML using comments to help make it easier to navigate. I also like how you used variables in your CSS, didn't see a lot of people do that in their coding solutions.

  However, I do have some suggestions for you. But these are just my opinion so feel free to take this on board or take it with a grain of salt, no hard feelings.

  In the HTML:

  • I'd recommend using an article tag for the nft preview component as opposed to a section tag. The article tag is a tag used for components that can be easily understood without any information around it. In otherwords you can look at it without it being in the context of the full site and it ill still makes sense. This allows it to be easily reusable and distributable. For example, blog posts, articles, components, widgets, etc. More on it here: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article
  • I'd recommend looking into BEM Naming Conventions for naming classes. It makes it easier for you to write your class names without having to think too much about it, and it also makes it easier for other developers to understand your classes, because the naming convention is quite a logical one. There are two articles I'd recommend reading on it, and once you read them you're pretty much set. The first one talks about how to write BEM (https://codeburst.io/understanding-css-bem-naming-convention-a8cca116d252 ) and the second one talks about common problems you may run into when writing BEM and how to overcome them (https://medium.com/fed-or-dead/battling-bem-5-common-problems-and-how-to-avoid-them-5bbd23dee319)
  • I'd recommend against using a div to display an image if the image is not purely decorative, i.e. doesn't provide any meaningful information to the user, because you can't add alt text to it. This will mean that visually impaired people will have no way of knowing that you even have images on your website.
  • Also when it comes to images I'd recommend using a picture tag instead of just the img tag because it's a much better way to add responsive images to your websites instead of just relying on CSS. CSS simply scales up and scales down an image, so if you use only one image to cater for the fact that it would be on both larger screens and smaller ones, you'd have to use a high-resolution image in your src for the image tag. However, not all users need that high-resolution image, and would prefer to load a lower-resolution one because their device might be slow as is, or they may just be using a device with a smaller screen and therefore loading a high-resolution file would be overkill, because even though it's scaled down it's still a high-resolution file meaning it's a large file to load. More on all that here: (https://blog.bitsrc.io/why-you-should-use-picture-tag-instead-of-img-tag-b9841e86bf8b).
  • Icons and images (unless it's purely decorative) should have alt text attached to it. The only other time you may not want alt text on your icons is when it's beside text that already essentially describes what it is, such as an icon of a happy dog beside text that says 'happy dog' because if you include the alt text the screen reader would end up reading out "happy dog, happy dog.”
  • Also when writing your alt text write it as though you're describing the image on the screen to a blind friend so they feel like they too can see it.
  • HTML has tags for almost everything, there's an address tag for example, and there's also a time tag. All geared towards helping browsers better understand your website, it also is helps with making your site more accessibile. With that said I'd recommend wrapping the '3 days left' in a <time></time> tag.

  In the CSS:

  • If you don’t give your HTML CSS, there is a default stylesheet applied to your HTML, and it’s not always ideal, so if you want a better default stylesheet to work with I’d recommend using a CSS reset. It provides you a much better foundation for you to build your CSS on top of. There are a few of them online, but the one I use at the moment is Josh W Comeau’s and the only addition I would add to it is to change the line-heights of the higher-level headings like the h1, h2 and h3 heading, as they tend to get a bit big at times. So, something like h1, h2, h3 {line-height: 1.1} should be good.
  • Another way you could center something in a container is by using something like display: grid; place-content: center to get the same result. You could also just as easily use display: flex and place-content: center on the parent element, it’s completely up to you. Also if this doesn’t give you the result you desire you could also try place-items instead of place-content, and if that doesn’t give you the results you want you could look into justify-content: center, and align-items: center.
  • I'd recommend naming your variables after their functions, not their specific names. This will allow you to re-use the same variables in different projects. Instead of writing something like --dark-blue, you’re better off pinpointing what the function of the colour is, whether it’s an accent colour (a colour used sparingly sort of like a highlight for certain elements of parts of elements), primary colour (main colours used in the UI) or a neutral colour (white, shades of gray black). Here is an article which goes into more depth: (https://m2.material.io/design/color/the-color-system.html#color-theme-creation) In doing so in the next project you can literally just copy and paste these variables and change the values because your project will more than likely have colours that function as primary, accent and neutral colours. The same can be done for typography. You could use the prefix fs for font-size and ff for font-family and fw for font. So, you could have something like fs-heading (for the heading font size), and you could do something like --ff-base and --ff-accent (for the base or regular font used on the site, and accent for things like headings), and finally something like --fw-light for lighter font-weights like 100, --fw-normal for font-weights like 400, --fw-bold for font weights like 700, etc. Writing your variables this way you also don't have to change their names from project to project all you have to really change is the value.
  • Also I'd recommend not using the hsl function in your color variables. You're better off just mentioning the values (i.e. '--soft-blue: 215, 51%, 70%' instead of '--soft-blue: hsl(215, 51%, 70%)'). This will allow you to later add an alpha value if needs be. For example, if you wanted the soft-blue colour to be slightly transparent, but used the variables the way you've written it, you wouldn't have that flexibility to do so. However, if you only used the values, then you could do something like this. 'color: hsl(var(--soft-blue), 50%)'

  Hope this helps:) And if you want me to explain anything a bit better just let me know, I was trying to keep this not too long, so I had to rush through some concepts. But like I said before, overall great work, and looking forward to seeing future builds.

  0