max-width and margins

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @itsgonenow,

  Congratulation on completing your first challenge here in frontend mentor. Your solution looks great. I have some suggestions regarding your solution if you don’t mind:

  • Use the <main> landmark to wrap the card, instead of <section> .HTML5 landmark elements are used to improve navigation experience on your site for users of assistive technology.

  • Page should contain <h1>. In this challenge , as it’s supposed to be a part of a whole page, you may use<h1>with sr-only class hidden visually and present for assistive tech users.

  • In my opinion , the alternate text is needed on this image. It should indicate where the Qr code navigate the user : like Qr code to frontend mentor

  CSS:

  • To center the component on the middle of the page you may use the flex/grid properties with min-height 100vh; to the body. Also you can add a little padding to the body to prevent the card from hitting.

  • height: 500px; It's not recommended to set height to component, let the content of the component define the height.

  • Consider using rem for font size .If your web content font sizes are set in absolute units, such as pixels, the user will not be able to re-size the text or control the font size based on their needs.Relative units “stretch” according to the screen size and/or user’s preferred font size, and work on a large range of devices.

  • Using percentages for widths , using max-width and avoiding to set heights for components, with these things is the key to mastering responsive layouts.

  Aside these , Good work! Hopefully this feedback helps

  Marked as helpful

  0