FrontendMentor-NFT-preview-card-component using Flex

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello @JemK-307 ,

  I have some suggestions regarding your solution:

  • There should be two landmark components as children of the body element - a main (which will be the card )

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

  • Page should contain a level-one heading, in this challenge , you can use <h1> instead of <h4> it would be like this : <h1> <a href="#"> Equilibrium #3429</a></h1>

  • Images must have alternate text

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

  • The alt text of the avatar shouldn't be empty " , it should be Jules Wyvern .Tips for writing 'good' alt text.

  • the icon-view 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:true 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.

  • You can use an unordered list<ul> to wrap class="card-info-sub" and in each <li>, there would be <img > and<p> (to wrap the text).

  • No need for <hr> , you can use border-top to the class="card-creator" .

  • No need for <span> , <p>Creation of <a href="#">Jules Wyvern<a></p>

  • It's better to use min-height: 100vh; for the body , using vh (viewport height) units to allow the body to set a minimum height value based upon the full height of the viewport.

    • an explicit width is not a good way . Remove the width from the card component and change it to max width instead. That will let it shrink a little when it needs to*/

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

  Overall your solution is good , Hopefully this feedback helps.

  Marked as helpful

  0

  ---

  

  Jem•30

  @JemK-307Posted almost 3 years ago

  @PhoenixDev22 Hi! Thank you so much for your useful and helpful feedback. These would be a great resource for me to keep practice harder and harder. really appreciate.

  1
• 

  Rio Cantre•9,690

  @RioCantrePosted almost 3 years ago

  Hello there! Great job in completing this project. Viewing at your solution, I would like to suggest the following for you...

  • Import the attribution style in the CSS file and remove the style tag
  • Wrap the content of container with specific tag like main. For HTML structures, refer it to this one Semantics
  • Include alt with descriptions to every images/svgs in img tags
  • Instead of using h4, alternative to use is h1 tag. Add font size with font-size: 22px;.
  • Adjust the size of the eye icon into width: 3rem; and position into left: 6.8em; in #view-icon rule set

  Above all, the design is looking great! Hope this helps and Keep it up!

  Marked as helpful

  0

  ---

  

  Jem•30

  @JemK-307Posted almost 3 years ago

  @RioCantre Hello! Thank you for your feedback. I'm newbie in the field of front-end. A new thing for me but it's fun. I will keep these feedback in my mind and practice harder.

  1
• Account deleted

  Hi there 👋

  Congratulate on finishing your project 🎉. You did a great job 🔨

  I give some suggestions that I hope help you take your project design to the next level 📈.

  • When I hover over the image the height of the card is changing. You can fix this issue 👍

  Happy coding ☕

  Maqsud

  0

  ---

  

  Jem•30

  @JemK-307Posted almost 3 years ago

  @maqsudtolipov Hi! Thank you so much for your feedback. I will try to fix it.

  1