Skip to content
Submitted over 2 years ago

NFT-preview-card-front-end-mentor HTML CSS

Bazthos•430
@Bazthos
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

If you can give me some tips and feedback about how to create div:hover shape the image (and note use margin % etc..)

Thank for all :)

Community feedback

  • PhoenixDev22•16,950
    @PhoenixDev22Posted over 2 years ago

    Hello @Bazthos ,

    I have some suggestions regarding your solution:

    • First of all , tables were never intended to be used for page layouts as it is Slow to render as the browser needs to download most - if not all - of the table to render it properly. for future , Use a table for tabular data and read more about the tables structures.

    • Use the main landmark to wrap the body content . HTML5 landmark elements are used to improve navigation .

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

    • In the avatar's alt shouldn't be avatar image , it's meaningless . you can use Jules Wyvern.

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

    • Read more how to write an alt text

    • There are so many ways to add the hover effect on the image , The one 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 element on hover as there is no reason to have the extra clutter . overlay in the html.

    • You can use <ul> to wrap the id="contain" , and in each <li> there would be <img > and <p>. then use the flexbox properties to aligh them centrally.

    • you can use <p> instead of <h2> , as they are not headers.

    • For the avtart's part , you may use <figure > and <figcaption>.

    • After you refactor you HTML, to center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the <body>.

    • using min-height: 100vh instead of height: 100vh allows the body to to grow taller if the content outgrows the visible page.

    • width: 300px; an explicit width is not a good way . consider using max-width to card instead. That will let it shrink a little when it needs to.

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

    • 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. General point : Really important to keep css specificity as low/flat as possible. The best way to do styling is single class selectors.

    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. Hopefuly this feedback helps.

    Marked as helpful
    2
    Bazthos•430
    @BazthosPosted over 2 years ago

    Hello @PhoenixDev22,

    Thank a lot for this detailed explanation, this will allow me to develop the right habits from the start. I'll follow the links and watch how you approached the problem so I can learn more about my mistakes.

    I wish you all the best in your future endeavors.

    1
  • Naveen Gumaste•10,480
    @NaveenGumastePosted over 2 years ago

    Hello Bazthos ! Congo 👏 on completing this challenge

    Let's look at some of your issues, shall we:

    • Bottom line can be more lighten? if can do so

    • Add font-weight to the "eth" and other text

    • Reduce the card border-radius a bit

    • Warp your card content around the main tag Ex: 👇

    <body>
<main class="container">
*all you content here*
</main>
</body>

    happy Coding😀

    Marked as helpful
    1
  • Travolgi 🍕•31,400
    @denieldenPosted over 2 years ago

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

    Also I have some tips for improving your code:

    • add main tag and wrap the card for Accessibility
    • not use table tag for the card but use div
    • not use inline style into html but create class in the css stylesheet
    • remove all margin from .conteneur class
    • try to use flexbox to the body for center the card. Read here -> best flex guide
    • after, add min-heigth: 100vh to body because Flexbox aligns to the size of the parent container

    Overall you did well :)

    Hope this help and happy coding!

    Marked as helpful
    1

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