Skip to content
Submitted almost 3 years ago

Html&Scss-nft-preview-card-component-main

itzjter•120
@itzjter
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

Solution retrospective

Hey! 's up? Please if you have any advice that can help me improve, i'm all ears. Thanks. :)

Community feedback

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

    Hello @itzjter,

    I have some suggestions regarding your solution:

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

    • 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 ( alt="ethereum icon", alt="clock icon", alt="eyes").

    • 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:true or role presentation with empty alt

    • The alt text of the shouldn't be `avatar `, it should beJules Wyvern ``.Tips for writing 'good' alt text

    • the link should be wrapping the original 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="info-row"and in each <li>, there would be <img > and<p>.

    • No need for <div class="divider"></div>, you can use border-top to theclass="user-info" .(in the html , there is <hr> do the same ).

    • <main> can be the card wrapper(no need for section )

    • After removing sectiontag and using <main> instead for the component ,To center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body>like this : (no need for the absolute position )

    body{
display:flex;
align-items: center;
justify-content: center;
width: 100vw;/*If you set a page width, choose 100% over 100vw to avoid surprise horizontal scrollbars.*/
width: 100%; 
min-height: 100vh;

    
.nft-card {

    width: 350px;/*    - an explicit width is not a good way . Remove the width from the card component and change it to ``max width`` instead for the card. That will let it shrink a little when it needs to.*/
    height: 610px;/*, you almost never want to set the height , let the content define the height.
*/
    padding: 24px;
    /* position: absolute; */
    /* top: 50%; */
    /* left: 50%; */
    /* transform: translate(-50%, -50%); */
    border-radius: 20px;
    box-shadow: 0 0 200px #000;
    background-color: #14253d;
}

    • Never use px for font-size.

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

    Overall your solution is good , Hopefully this feedback helps.

    Marked as helpful
    2
    itzjter•120
    @itzjterPosted almost 3 years ago

    @PhoenixDev22 Wow! Thank you very much for your help, i will always remember it. ^^

    1
  • Anosha Ahmed•9,300
    @anoshaahmedPosted almost 3 years ago

    Hey good job on this challenge! You should have at least one <h1> in your code

    Here's a link to something I wrote about accessibility issues & best practices

    Hope this helps :)

    Marked as helpful
    1
    itzjter•120
    @itzjterPosted almost 3 years ago

    @anoshaahmed okay, thanks :)

    1
  • Charlie•360
    @Charlie025xPosted almost 3 years ago

    Great solution! I took a peak at your code to see how you did the cyan overlay and learned alot from how you utilized opacity with scss.

    Marked as helpful
    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