NFT Preview Card Component

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted almost 3 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 almost 3 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

  

  T

  Grace•31,270

  @grace-snowPosted almost 3 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 almost 3 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 almost 3 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