NFT Preview Card Component

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello romila,

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

  HTML

  1. Since there's a :hover state on the image and means it's interactive, So there should be an interactive element around it. When you create a component that could be interacted with a user , always remember to include interactive elements like(button, textarea,input, ..) for this imagine what would happen when you click on the image, there are two possible ways:
     1: If clicking the image would show a popup where the user can see the full NFT, here you use <button>. 2:If clicking the image would navigate the user to another page to see the NFT, here you can use <a>. For the same reason , you can use <a> to wrap Equilibrium #3429 and Jules Wyvern.

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

  3. 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 ).

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

  5. For class="same-line", you can use an unordered list <ul>, in each <li> , there should be <img> and <p> that way you can align them centerally.

  6. If you wish to draw a horizontal line, you should do so using appropriate CSS. You may remove the <div class="border-bottom"></div>, you can use border-top: to the avatar's part.

  7. To use more semantic tags , you may use <figure> and <figcaption> for the avatar's part.

  CSS:

  1. Consider using rem for font size. If your web content font sizes are set in absolute units, such as pixels, the user will not be able to re-size the text or control the font size based on their needs. Relative units “stretch” according to the screen size and/or user’s preferred font size, and work on a large range of devices.

  2. As the image is not display: block , likely that's why you needed to use height: 98% to the pseudo element.

  3. Remember a css reset on every project. That will do things like set the images to display block and make all browsers display elements the same.

  Overall , your solution is good. Hopefully this feedback helps.

  Marked as helpful

  1

  ---

  

  romila•3,570

  @romila2003Posted over 2 years ago

  @PhoenixDev22 Thank you for the feedback, I really appreciate it. I took those suggestions into consideration and changed my code and have updated it so thanks again.

  1

  

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  @romila2003

  Hi Romila again, You're welcome , Glad it was helpful. I have looked into your solution again. By the way , Every developer has his own vision,

  -1. I would think that the <h1><a>..</a></h1> approach is more conventional though that way only the text in the title will be clickable.

  -2. As you chose a button for the NFT image, so the alt text should say what the button does.

  -3. Also , I would not use <section> for only a part of the component as all the content of the card is related .

  Hopefully this helps. Happy coding

  Marked as helpful

  0

  

  romila•3,570

  @romila2003Posted over 2 years ago

  @PhoenixDev22 Hi thanks again for the suggestion but I have some questions.

  1. When you mentioned to include the alt text, is it necessary to include it within the button tag? And if so, what text do you think should be inside it because I'm not sure what to write regarding what the button does.

  Sorry, if it's a simple question but I never added an alt for a button before so I was just wondering.

  Thanks again for the help.

  1

  

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  @romila2003

  Hi romila, Happy to help and Asking questions is crucial in learning.

  I meant the alternate text of the image not within the button and it should indicate the purpose of the button what the button does, like: see the full NFT.

  Within the button you may use aria-label attribute to make accessible as NFT button contains an image and no (visible) accompanying text. I would suggest to look up on how to make accessible buttons.

  Happy coding!

  Marked as helpful

  1