NFT Preview Card Component Solution

Community feedback

• 

  Naveen Gumaste•10,480

  @NaveenGumastePosted almost 3 years ago

  Hay ! Peter Good Job on challenge

  These below mentioned tricks will help you remove any Accessibility Issues

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

  -> Learn more on accessibility issues

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

  Have a good day and keep coding 👍!

  Marked as helpful

  0

  ---

  

  Peter Hannell•200

  @peterhannellPosted almost 3 years ago

  @Crazimonk,

  Thanks for your feedback, much appreciated!

  1
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello @peterhannell ,

  I have some suggestions regarding your solution:

  To tackle the accessibility issues:

  • There should be two landmark components as children of the body element - a main (which will be the NFT card ) and a footer (which will be the attribution).<Footer> should be in the <main >read more about A simplified web page, might look something like this:

  • Anything with a hover style in a design means it's interactive. you need to add an interactive element around the image Equilibrium #3429, Jules Wyvern.

  It would be better

  <h1><a href="#">Equilibrium #3429</a></h1>
  <p>Creation of <a href="#">Jules Wyvern</a></p>
  
  

  • 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="Icon View" alt="Ethereum", alt="Clock"

  • 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 alt="Avatar" You can use Jules Wyvern as an alt text.

  • You can read more about alt text

  • You can use an unordered list <ul> for class="flex"and in each <li>, there would be <img> and<p>(for the text ).

  • You can use border-top to the class="card__footer">instead of <hr/>.

  • Section lacks heading. Consider using h2-h6 elements to add identifying headings to all sections.(even though no need for a <section> in this challenge)

  • I don't recommend to change the base html size. This has huge accessibility implications for those of us with different font size or zoom requirements.

  
  html {
    font-size: 62.5%;
  }
  
  

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

  • Using percentages % for width can cause a lot of problems , consider using max-width. That will let the component grow up to a point and be limited.

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

  Marked as helpful

  0

  ---

  

  Peter Hannell•200

  @peterhannellPosted almost 3 years ago

  @PhoenixDev22,

  Thanks! That's really helpful. I will incorporate your suggestions into my solution.

  1
• 

  Rio Cantre•9,690

  @RioCantrePosted almost 3 years ago

  Hello there! Nice work with this one. Looking at your solution, I would recommend the following for you...

  • Remove margin-top: 1rem; in .card__desc p:first-of-type rule set to align the elements in .flex
  • Remove unnecessary code to keep it clean

  Above all, you did really well. Keep it up!Cheers!

  Marked as helpful

  0

  ---

  

  Peter Hannell•200

  @peterhannellPosted almost 3 years ago

  @RioCantre,

  Thank you! I appreciate the feedback and suggestions.

  0