Responsive qr component using HTML & CSS

Community feedback

• 

  Daniel 🛸•44,370

  @danielmrz-devPosted 12 months ago

  Hello @MombinyaOmboga

  Your solution looks great!

  I have a couple of suggestions for improvement:

  • In order to make your HTML code more semantic, and since that is the main title of the screen, you can replace the <h5> with <h1>. Unlike what most people think, it's not just about the size and weight of the text.

  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, still about semantic HTML, you can replace your div.img-container with main.img-container.

  All these tag changes may have little or no visual impact but they make your HTML code more semantic and improve SEO optimization as well as the accessibility of your project.

  I hope it helps!

  Other than that, great job!

  Marked as helpful

  0
• 

  Melvin Aguilar 🧑🏻‍💻•61,120

  @MelvinAguilarPosted 12 months ago

  Hello there 👋. Good job on completing the challenge !

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

  Background 🌆:

  • You should not recreate the background, you used the image desktop-preview.jpg to create your solution, but that image is for decoration and is a nice way to present the challenge, for example, you can use it in your github README.

    You must use the images desktop-design.jpg and mobile-design.jpg to create your solution.

  HTML 🏷️:

  • Wrap the page's whole main content in the <main> tag.

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

  Happy coding!

  Marked as helpful

  0

  ---

  

  Mombinya Omboga Shem•170

  @MombinyaOmbogaPosted 12 months ago

  @MelvinAguilar thank you for the suggestion on my code improvement, I'm just getting to learn frontend, as such would you explain why I need to wrap the whole content in a <main > tag. I omitted using a <main> tag, given the “simplicity” of the design. Thanks for taking your time to review and answer.

  0

  

  Mombinya Omboga Shem•170

  @MombinyaOmbogaPosted 12 months ago

  @MelvinAguilar also for the design, I think I will use the desktop-design.jpg for practice, using the desktop-preview as also a nice way to practice other design concepts I had learned.

  0
• 

  Haydn Kerr•535

  @haydnkerrPosted 12 months ago

  Nice final design! Since the design calls for a card to be placed directly in the middle of the viewport a good way to do this one is to skip the .img-container and do all the positioning in the body tag. body { display: flex; justify-content: center; align-items: center; height: 100svh; width: 100% }

  this will place the QR card directly in the middle without having to do any further positioning on the .img-holder class

  Happy Coding!

  0