Skip to content
Submitted almost 3 years ago

NFT preview card component using Flexbox

Renato Navarro Alberco•100
@GintokiYT
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...NFT preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Community feedback

  • PhoenixDev22•16,950
    @PhoenixDev22Posted almost 3 years ago

    Hello @GintokiYT,

    I have some suggestions regarding your solution:

    • You are misusing the <section> tag . section is not meant to b e used anytime you feel tempted to use a div . section is for a bigger chunk of content often titled by <h2> Read more aout usage notes

    • Anything with a hover style in design means it's interactive. you need to add an interactive element<a> around the image Equilibrium #3429 and Jules Wyvern .

    • For any decorative images, each img tag should have empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images in( icon-view, icon-ethereum, icon-clock ).

    • the icon-view doesn't really need to be in the html, you could do it with css. If you want it to stay in html it needs to be aria-hidden or role presentation with empty alt.

    • I would use pseudo-elements . You either have to change the teal bg color to a hsla. Then opacity can be changed from 0 to 1 on the pseudo on hover.

    • The avatar 's alt should not be avatar , it's meaningless. you can set Jules Wyvern to it.

    • Read more how to write an alt text

    • the link should be wrapping the main image and either have Sr-only text, an aria-label or alt text that says where that link takes you.

    • It' better not to use <span> and div for meaningful content .

    • You can use an unordered list<ul> to wrap class="container__text-price"and in each <li>, there would be <img > and<p> .

    • Use min-height: 100vh for the <body> . Gather the <body> styles in one body{}

    min-height: 100vh;/*using vh (viewport height) units to allow the body to set a minimum height value based upon the full height of the viewport.This also allows the body to to grow taller if the content outgrows the visible page.*/
    • You should use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.

    You might have a look at my solution , to see how I did the hover effect on the image , it might my helps.

    Overall, your solution is good, Hopefully this feedback helps.

    0
  • Travolgi 🍕•31,400
    @denieldenPosted almost 3 years ago

    Hi Renato, I took some time to look at your solution and you did a great job!

    Also I have some tips for improving your code:

    • remove all margin from .container class because with flex they are superfluous
    • add a little transition on the title and user name with hover effect

    Overall you did well :)

    Hope this help and happy coding!

    0

Please log in to post a comment

Log in with GitHub

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