NFT preview card component - my 3rd challenge

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted about 2 years ago

  Hello Julien,

  Congratulation on completing this challenge. Excellent work! I have few suggestions regarding your solution, if you don't mind:

  HTML

  • You can use the <main> landmark to wrap the NFT. And <footer> landmark to wrap the attribution, using landmarks is important to improve navigation experience on your site for users of assistive technology.

  <a href="#"><h1></h1></a>
  <h1 ><a href="#"></a></h1>
  

  they don't have the exact same effect. So it depends on what you want to make a hyperlink, in case you surround the entire<h1>you need to put it as a parent. In case you want to get only a specific part of the <h1> you only surround that part like this:<h1 ><a href="#"></a></h1>.

  • The link wrapping the equilibrium image should either have Sr-only text, an aria-label or alt text that says where that link takes you.

  • For any decorative images, each img tag should have empty alt="" and add aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images in (icon-view, icon-ethereum, icon-clock ).

  • Profile images like that avatar are valuable content. The alternate text of the avatar’s image should not be author-avatar-img. You can use the creator's name Jules Wyvern. Read more how to write an alt text

  • If you wish to draw a horizontal line, you should do so using appropriate CSS. You may remove the <hr>, you can use border-top: to the avatar's part.

  • look up a bit more about how and when to write alt text on images. Learn the differences with decorative/meaningless images vs important content

  • There are so many ways to do the hover effect on the image, The one I would use is pseudo elements::before, ::after. You can use pseudo-elements to change the teal background color to hsla. Then the opacity can be changed from 0 to 1 on the pseudo element on the hover. Also using pseudo elements makes your HTML more cleaner as there's no need for extra clutter in the HTML.

  • Adding rel="noopener" or rel="noreferrer" totarget="_blank"links. When you link to a page on another site using target=”_blank” attribute , you can expose your site to performance and security issues.

  Aside these, Great job on this one. Hopefully this feedback helps.

  Marked as helpful

  0
• 

  Ayush Kumar•10

  @blazingfringePosted about 2 years ago

  You can wrap all your body inside main to get rid of that accessibility issue in the solution report.

  0
• 

  Razha Jamsik Syah•700

  @Razha90Posted about 2 years ago

  • position: absolute; is best solution for this , but you give set position like top or left , if you delete set position and give little margin for setting position you got it .
  • This card not centering , you can add body {width:100vh; height: 100vh; display:flex; align-items: center; justify-content: center;} for make card be auto center. you can deletetemargin: auto; and .content-and-attr

  Happy Coding 😆

  0

  ---

  

  Julien•40

  @windu80Posted about 2 years ago

  @Razha90 Thanks for that! I changed the top and left, and use the margin instead, it works :)

  For the rest, I haven't learnt Flexbox yet, so I'll come back to that later

  0