Used flex to center the component

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hi Mateo Sarubbi,

  Congratulation on completing this frontend mentor challenge. Your solution looks great. I have some suggestions regarding your solution:

  • You should use <main> for the card. 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> visually hidden with class=”sr-only”. You can find it here

  • Consider using rem and em units as they are flexible, specially for font size better to use rem. 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

  • Remember a css reset on every project. That will do things like set the images to display block and make all browsers display elements the same.

  Overall, very clean code and well-structred ! Hopefully this feedback helps.

  Marked as helpful

  1
• 

  Adriano•39,190

  @AdrianoEscarabotePosted over 2 years ago

  Hello everything is fine?

  You really did a great job on this challenge, everything is very good, especially the responsiveness of the project. I really liked the way you wrote your CSS.

  I have a tip about HTML:

  1- Document should have one main landmark, you could have put all the content inside the main tag click here

  2- Page should contain a level-one heading click here to read about it

  The rest is really good! Hope it helps... 👍

  Marked as helpful

  1
• 

  David•8,000

  @DavidMorgadePosted over 2 years ago

  Hello Mateo, congrats on finishing the challenge, great job getting your component centered!

  I just have some suggestions that can make your code a little bit cleaner.

  Why you have a margin on your div class='component'? you should remove it because its doing nothing since your component is already centered, giving a margin from both sides will have no effect at all.

  Also, use the main text of your component as an h1 tag since is the most important heading in this case, you could also wrap your component on a main tag instead of a div to give it a more semantic meaning and pass the test validations from Frontendmentor.

  I would also not recommend setting the font-size of the body to 15px, leave it as the default 16px and add your sizes to each tag!

  Apart from that, great job with your component, hope my feedback helps you!

  Marked as helpful

  1
• 

  Luigi•240

  @luigi-peronePosted over 2 years ago

  Hi Mateo, welcome to the Frontend Mentor community 👍

  You've a great solution here but there are some tags you can consider changing in the html, for example, the h2 should be wrapped with an h1, you can also replace the first div with an main tag and you can change the div with attribution class as a footer tag. These changes don't change the design but improve the accessibility and semantic.

  Hope it helps, happy coding 👋

  Marked as helpful

  1