Responsive page - using Flexbox and positioning

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @David-Paulos ,

  Congratulation on completing this frontend mentor challenge.

  I have some suggestions regarding your solution:

  • To tackle the accessibility issues: Page should contain a level-one heading . In this challenge , you can use <h1> with class="sr-only" (Hidden visually, but present for assistive tech).And use a <h2> for the Equilibrium #3429 (a header not <p> )

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

  • There are so many ways to add the hover effect on the image , The one I would use, using pseudo-elements to change the teal bg color to a hsla. Then opacity can be changed from 0 to 1 on the pseudo element on hover as there is no reason to have the extra clutter in the html.

  The link wrapping the equilibrium image should either have Sr-only text, an aria-label or alt text that says where that link takes you.

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

  • look up a bit more about how and when to write alt text on images. Learn the differences with decorative/meaningless images vs important content

  CSS

  • Changing base html size has huge accessibility implications for those of us with different font size or zoom requirements.

  • To center the card on the middle of the page use flexbox properties and min-height to the body. And remove the position absolute from the card.

  body {
  ....
  display: flex;
      align-items: center;
      justify-content: center;
      min-height: 100vh;
  }
  
  

  • Use ``min-height: 100vh; to the body 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: 260px;an explicit width is not a good way . consider using max-width to card instead and a little margin to the card .That will let it shrink a little when it needs to

  • height: 430px;It's rarely ever a good practice to set heights on elements . Let the content inside the card element dictate the height of it.

  General point :

  • It's recommended 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.

  • Following the challenge , I would suggest to leave the avatar' part and the attribution should be out the**NFT* card.

  • Add transition to the hover elements

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

  Marked as helpful

  1

  ---

  

  David Paulos•80

  @David-PaulosPosted over 2 years ago

  @PhoenixDev22 Thank you very much for your feedback, it is much appreciated !! have a look at a new challenge I completed.

  Let me know your thoughts !!

  0