HTML, CSS

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @DarilInsanKamil ,

  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 component ) and a footer (which will be the attribution).<Footer> should not be in the <main >. HTML5 landmark elements are used to improve navigation .

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

  • Page should contain a level-one heading . In this challenge , you can use <h1> for Equilibrium #3429 instead of <p > as you should have used a header .

  • 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-ethereum, icon-clock

  • The avatar's alt shouldn't be **profile ** , it's meaningless . you can use Jules Wyvern.

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

  • Read more how to write an alt text

  • You can use <ul> to wrap the class="minting" , and in each <li> there would be <img > and <p>. then use the flexbox properties to aligh them centrally.

  • No need for all this <div class="separator"></div>, as you simply can use ``border-top: to the.class="profile"`

  • For the avatar 's part , you may use <figure > and <figcaption>.

  • Using widths in percentages , makes you lose the control of the layout , consider using max-width: 21.875; instead and a little padding to the body or margin to card that will make the card shrinks a little when it's needed.

  • using min-height: 100vh instead of height: 100vh allows the body to to grow taller if the content outgrows the visible page.

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

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

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

  1
• 

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  Hi Daril, I took some time to look at your solution and you did a great job!

  Also I have some tips for improving your code:

  • add main tag and wrap the card for Accessibility
  • To make it look as close to the design as possible set width: 20rem; to card class
  • You can add the effect :hover creating a div that appears on hover. I used tailwind but you can still see and understand which css properties you can use to do the same. Look here -> my solution
  • after try to add a little transition on the element with hover effect

  Overall you did well :)

  Hope this help and happy coding!

  0