HTML, CSS, Flexbox, git, github

Community feedback

• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted 11 months ago

  Hello there 👋. Good job on completing the challenge !

  I have some suggestions about your code that might interest you.

  HTML 🏷️:

  • Use the <footer> tag to wrap the footer of the page instead of the <div class="attribution">. The <footer> element contains information about the author of the page, the copyright, and other legal information.

  CSS 🎨:

  • For a responsive and resizable component, consider using max-width: 320px or max-width: 20rem for the width and adding margin to avoid border touching the border of the screen in mobile devices. Also, remove the large padding in the text elements (/*padding: 0 5em;*/)

  • The color of the paragraph element is unreadable because it is too light compared to its background. For example, change the color to hsl(220deg, 15%, 55%).

  • To center the component in the page, you should use Flexbox or Grid layout. You can read more about centering in CSS here 📘.

  I hope you find it useful! 😄 Above all, the solution you submitted is great!

  Happy coding!

  Marked as helpful

  1

  ---

  

  P

  cloudpc7•180

  @cloudpc7Posted 11 months ago

  @MelvinAguilar

  thank you very much for the insight. I did another one and improved upon the feedback. Thank you

  0
• 

  Daniel 🛸•44,310

  @danielmrz-devPosted 11 months ago

  Hello @cloudpc7!

  Your solution looks great!

  I have a couple of suggestions for improvement:

  • For semantic reasons, and since that is the main title of the project, you can replace the <h2> with <h1>.

  The <h1> to <h6> tags are used to define HTML headings. <h1> defines the most important heading. <h6> defines the least important heading. Only use one <h1> per page - this should represent the main heading/subject for the whole page. Also, do not skip heading levels - start with <h1>, then use <h2>, and so on.

  • Also, I noticed that you used margin to place the card in the middle of the page. Here's a very efficient way to center the card that adapts better with screen size changes:

  You can apply this to the body (in order to work properly, you can't use position or margins):

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

  I hope it helps!

  Other than that, great job!

  0

  ---

  

  P

  cloudpc7•180

  @cloudpc7Posted 11 months ago

  @danielmrz-dev

  thank you very much for the insight. I did another one and improved upon the feedback. Thank you

  0