flexbox

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello @Tham-Bahadur-Pun,

  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 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:. There is an extra div

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

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

  • 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 or role presentation with empty alt.

  • the avatar 's alt shouldn't be "avatar image" it's meaningless , you can set to Jules Wyvern .Also , the main image alt needs to be changed.

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

  • You can use an unordered list<ul> to wrap class="inner-content and in each <li>, there would be <img > and<p> .

  • Add display: block ; to the image to remove the slight gap I see under the image on mobile.

  • No need for <hr class="horizontal-line">, you can use border-top: to the avatar's part.

  • I recommend to 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.

  • Never use px for font size.

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

  • min-height: 100vh; using vh (viewport height) units to allow 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.

  You might have a look at my solution , to see how I did the hover effect on the image , it might my helps.

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

  Marked as helpful

  0

  ---

  

  Tham Bahadur Pun•190

  @Tham-Bahadur-PunPosted over 2 years ago

  @PhoenixDev22 thanks for your suggestion it was wonderful.

  0
• 

  Naveen Gumaste•10,480

  @NaveenGumastePosted almost 3 years ago

  Hay ! Tham Bahadur Pun Good Job on challenge

  These below mentioned tricks will help you remove any Accessibility Issues

  -> Consider using h2-h6 elements to add identifying headings to all sections.

  -> 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
• Account deleted

  Hello there! 👋

  Congratulations on finishing your challenge! 🎉

  I have some feedback on this solution:

  put the .container div inside <section> tag to be more setmantic

  i hope this is helpful

  Marked as helpful

  0
• 

  Kamrul Hassan•100

  @khshakibPosted almost 3 years ago

  great job

  0