Simple Card

Community feedback

• Account deleted

  Hi there 👋

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

  I give some suggestions to help you take your project design to the next level 📈😉

  • When I hover on Equilibrium #3429 and Jules Wyvern texts their color should change to 00fff7
  • Also adding height is not good practice in CSS in most cases, it should be calculated automatically, so remove the height from the card. Don't worry it still works ✅
  • Also don't forget to add some box-shadow to it

  Happy coding ☕

  Maqsud

  Marked as helpful

  0

  ---

  

  Huy Vo•340

  @bmhuyquoc104Posted over 2 years ago

  @maqsudtolipov Thank you so much, I will follow your advice to make it better !!

  1
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @bmhuyquoc104 ,

  I have some suggestions regarding your solution:

  • First o all , I recommend to use a separate file for the styles.

  • To tackle the accessibility issues, you can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech).

    • 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 and the hover effect on image.

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

  • the avatar 's alt shouldn't be empty , you can set to Jules Wyvern.

  • Read more how to write an alt text

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

  • Add the icon-view on the hover , 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

  • You can use an unordered list<ul> to wrap class="info" and in each <li>, there would be <img > and<p> (to wrap the text ).Remove the <h3>don't use span for meaningful content. Then you can use flexbox properties to align them centrally.

  • <img src="./images/icon-ethereum.svg" alt="" srcset="">why did you use srcset attribute here>

  • Use min-height: 100vh; instead of height: 100vh this allows the body to set a minimum height value based upon the full height of the viewport. This also allows the body to to grow taller if the content outgrows the visible page

  • width: 300px; an explicit width is not a good way . Remove the width from the main component and change it to max width instead. That will let it shrink a little when it needs to.

  • For this don't use div for meaningful content , it would be like<p class="name">Creation of <a href="#" class="" >Jules Wyvern</a> </p>

  A general point - Never use px for font size.

  You might have a look at my solution , it might my helps.

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

  0

  ---

  

  Huy Vo•340

  @bmhuyquoc104Posted over 2 years ago

  @PhoenixDev22 Thank you so much for your advices ! I will follow it to make it looks better

  1
• 

  optimusprime202•1,200

  @optimusprime202Posted over 2 years ago

  Hey @bmhuyquoc104, Outstanding work!

  0