NFT Preview Card | Flexbox

Community feedback

• 

  Luka•820

  @LukaKobaidzePosted almost 3 years ago

  Hello!

  To remove extra space to the container, you can add display: block to the image with the class of nft-img

  If you want to know why this happens and why we add display: block to the image, check out this

  Marked as helpful

  2

  ---

  

  shuberber•230

  @sheronimoPosted almost 3 years ago

  @LukaKobaidze Hi Luka! I completely forgot about img elements being inline, changing it to display: block did the trick!

  Thank you very much! 🌹

  1
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello @sheronimo ,

  I have some suggestions regarding your solution:

  • for the image hover, first it should be wrapped in an anchor tag as already stated. You can add either aria label or Sr onlytext oralt`` text on the image that says where the link goes.

  • The eye image doesn't really need to be in the html, you could do it with css.

  • You can use an unordered list <ul> to wrap class="nft-info"and in each list item would be <img> and <p>.

  • No need for the <hr class="divider" />, you can use border-top: ; to the class="nft-author" . Also , to use more semantic tags , you can use <figure > and <figcaption> for class="nft-author"`.

  Hopefully this feedback helps.

  Marked as helpful

  1

  ---

  

  shuberber•230

  @sheronimoPosted almost 3 years ago

  @PhoenixDev22 Hi Phoenix, thanks for all the suggestions, I really appreciate it! Great pointers for my markup (and thus, styling) to be more efficient.

  Definitely will work on these in my continued development, and in future projects as well. 🌹

  0
• 

  Naveen Gumaste•10,480

  @NaveenGumastePosted almost 3 years ago

  Hay ! shuberber Good Job on challenge

  These below mentioned tricks will help you remove any Accessibility Issues

  -> Always use the "alt attribute" and just leave it empty bt.. always add it with img tag.

  -> Learn more on accessibility issues

  Keep up the good work!

  Marked as helpful

  1

  ---

  

  shuberber•230

  @sheronimoPosted almost 3 years ago

  @Crazimonk Hi Naveen, thank you for the tip! I made sure to include the alt attribute in the smaller icons too in my update. 👍

  1
• 

  Travolgi 🍕•31,400

  @denieldenPosted almost 3 years ago

  Hi Shuberber, great work !

  Add aspect-ratio: 1; for exact same size to the .overlay class

  And don't forget img element need alt attribute, it's very important

  Hope this help ;)

  Marked as helpful

  1

  ---

  

  shuberber•230

  @sheronimoPosted almost 3 years ago

  @denielden Hi Deniel,

  I decided to use a display: block on the image, since I completely overlooked it being an inline element, but thank you for this suggestion! It's a new method for me to experiment with 😁

  Thanks too for the heads-up on the alt attributes! 🌹

  1