NFT preview card component

Community feedback

• 

  PhoenixDev22•16,830

  @PhoenixDev22

  Posted over 3 years ago

  Hello, I have some suggestions regarding your solution: To tackle the accessibility issues:

  • Wrap the body content in <main> . a <footer > for the attribution and it should be out of the <main>
  • Page should contain a level-one heading
  • you can add a<h1>with class="sr-only" (Hidden visually, but present for assistive tech).

  .sr-only {
  	border: 0 !important;
  	clip: rect(1px, 1px, 1px, 1px) !important;
  	-webkit-clip-path: inset(50%) !important;
  		clip-path: inset(50%) !important; 
  	height: 1px !important;
  	margin: -1px !important;
  	overflow: hidden !important;
  	padding: 0 !important;
  	position: absolute !important;
  	width: 1px !important;
  	white-space: nowrap !important;            
  }
  

  • Anything with a hover style in a design means it's interactive.you need to add an interactive element which is <a> around the image and Equilibrium #342 and Jules Wyvern

  • For any decorative images, each img tag should have empty alt="" andaria-hidden="true"attributes to make all web assistive technologies such as screen reader ignore those images in alt="clock icon"" , alt="diamond icon".

  • the eye image doesn't really need to be in the html, you could do it with css. If you want it to stay in html it needs to be aria-hidden or role presentation with empty alt

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

  • The avatar's alt text shouldn't be only alt="author picture". You can use Jules Wyvern as an alt text.

  • Never use px for font-size.

  • You should use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.

  • The card has an explicit width. Remove the width from the card and change it to max width: 350px; instead. That will let it shrink a little when it needs to, It's rarely ever a good practice to set heights height: 560px;on elements . let the content define the height.

  • The hover effect on the image is not set properly.

  Hopefully this feedback helps.

  21

  Marked as helpful
• Account deletedPosted over 3 years ago

  Hello there! 👋

  Congratulations on finishing your challenge! 🎉

  I have some feedback on this solution:

  • Always Use Semantic HTML instead of div like <main> <header> , etc for more info.

  • you can add the image and background color on hover with pseudo elements :before & ::after if you are not sure what i am talking about you can learn from my example.

  if my solution has helped you do not forget to mark this as helpful!

  10

  Marked as helpful
• 

  Naveen Gumaste•10,420

  @NaveenGumaste

  Posted over 3 years ago

  Hay ! Łukasz Good Job on challenge

  These below mentioned tricks will help you remove any Accessibility Issues

  -> Add Main tag after body <main class="container"></main>

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

  -> Learn more on accessibility issues

  If this comment helps you then pls mark it as helpful!

  Have a good day and keep coding 👍!

  00

  Marked as helpful
• 

  Kacper Kwinta•1,405

  @kacperkwinta

  Posted over 3 years ago

  Some tips:

  • Page should contain a level-one heading
  • Images must have alternate text
  • create <main> tag and wrap all other into this (semantic and accessibility), more about this here

  00

  Marked as helpful