Skip to content
Submitted over 3 years ago

NFT-Card-Preview-Using-CSS-Bootstrap

bootstrap, accessibility
David Janak•20
@JanakDavid
A solution to the NFT preview card component challenge
Preview (opens in new tab)Code (opens in new tab)
Code
Couldn’t fetch repository

Please log in to post a comment

Log in

Community feedback

  • PhoenixDev22•16,830
    @PhoenixDev22

    Hello @JanakDavid ,

    I have some suggestions regarding your solution:

    • There should be two landmark components as children of the body element - a main (which will be the NFT card ) and a footer (which will be the attribution).<Footer> should be outside the <main >. HTML5 landmark elements are used to improve navigation .

    • You are misusing the <section> tag . section is not meant to be 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 avatar's alt shouldn't be creator-img , you can use Jules Wyvern instead .

    • 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 to change the teal bg color to a hsla. Then opacity can be changed from 0 to 1 on the pseudo on hover as there is no reason to have the extra clutter in the html.

    • 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.

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

    • No need for <hr> . You can use border-top to `creator-sign``.

    • To center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body>( noo need for position absolute .)like this:

    body{
display:flex;
align-items: center;
justify-content: center;
width: 100%; 
min-height: 100vh;

    • width: 21.875rem;an explicit width is not a good way . Remove the width from the main component and change it to max width instead. That will let it shrink a little when it needs to.

    • height: 37.25rem; It's rarely ever a good practice to set heights on elements . Let the content inside the card element dictate the height of it.

    • Never use px for font size .You should set the font-size on the element using rem unit. Using px won't allow the user to control the font size based on their needs.

    • You should use em and rem units .

    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.

    Marked as helpful
  • Travolgi 🍕•31,280
    @denielden

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

    Also add main tag and wrap the card for Accessibility and remove all margin from eth-box id because with flex they are superfluous and add align-items: center; for center the card vertically.

    Overall you did well :)

    Hope this help and happy coding!

    Marked as helpful

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