Skip to content
Submitted over 2 years ago

NFT Preview Card Component

selehadin cyber•90
@selehadin-cyber
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

Hii everyone

this challenge was both fun and ... a bit challenging. and it was my second challenge a couple of question that i have are

  1. the text in my p element looks bigger than the one in the design folder even though i am using the stated font size for paragraph which is 18px

2)the element that appears when hovering over the image doesn't cover the image under it perfectly. in spite of me trying width 100% and height 100% so i had to resort to position it absolutely incrementing or decrementing pixel by pixel until i was decently satisfied. is there an easier way to do this

  1. is absolute positioning frowned upon? i feel a bit guilty when using absolute positioning instead if trying to align it with flexbox or grid.

4)i feel like i have already used too many divs is this normal or against best practice?

Community feedback

  • Grace•30,990
    @grace-snowPosted over 2 years ago

    hi

    Anchor tags shouldn't wrap a heading tag, mswap those around so the anchor is inside.

    • the main img needs to be display block (that will solve the size issue you mentioned)
    • you need to have an anchor tag wrapping the main image. At the moment the anchor doesn't wrap the main image
    • label on that anchor with accessibile text - either aria label or Sr only text or alt text on the image that says where the link goes
    • the svgs youve put inline could be in img tags. If using inline svgs you still need yo give them a role of img and either aria hidden or alt text. I think they should be hidden.
    • I would do the image hover with absolutely positioned pseudo elements as there is no reason to have the extra clutter in the html

    Last thing - use min height 100vh not height! This is breaking your solution on mobile landscape atm

    Hope this helps

    Marked as helpful
    1
    selehadin cyber•90
    @selehadin-cyberPosted over 2 years ago

    @grace-snow

    Hi grace

    that was a very helpful feedback specially the last to points putting svgs in img tags would make my code much more cleaner. didn't know that was possible till very recently

    • pseudo elements are also a neater way but how do i put the svg in the pseude element. do i just put the url in the content: "" ?
    0
    Grace•30,990
    @grace-snowPosted over 2 years ago

    @selehadin-cyber background image, leave content blank.

    If using pseudo approach You either have to change the teal bg color to a hsla (a is the alpha value, makes it opaque) if using one pseudo. Then opacity can be changed from 0 to 1 on the pseudo on hover. Or you have to use two pseudos, one for the teal color and one for the eye image. Then you can manage the hover opacity of each separately

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

    Hi selehadin ,

    absolute positioning is not frowned upon but should be used when appropriate without abusing too much.

    The amount of div you used is fine ... always keep in mind that the best practice is to write as little code as possible.

    Hope this help :)

    Marked as helpful
    1
    selehadin cyber•90
    @selehadin-cyberPosted over 2 years ago

    @denielden

    Hi daniel,

    thanks a lot for the feedback it did help... i will try to use absolute positioning as a last resort inshaAllah👍

    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