QR Code Challenge using HTML and CSS

Community feedback

• 

  Deepanshu Gupta•590

  @Deepanshu-5288Posted about 2 years ago

  Hello David, It's looking great just a little suggestion.

  Since you are using flex you can easily center your card using flex properties rather than using margin. You can refer to the below link: https://www.w3schools.com/css/css3_flexbox_container.asp

  Make sure adding min-height : 100vh as it will make your div have height of the 100% of the view height so that when you use align-content:center it will be in center of 100vh otherwise height will be adjusted to the max height of the content inside div which will not be in the center of screen.

  I hope its help you.

  Thanks, Deepanshu

  Marked as helpful

  1
• 

  Lucas 👾•104,340

  @correlucasPosted about 2 years ago

  👾Hello David, congratulations on your first solution!👋 Welcome to the Frontend Mentor Coding Community!

  Nice code and nice solution! You did a good job here putting everything together. I’ve some suggestions for you:

  1.Use <main> instead of a simple <div> this way you improve the semantics and accessibility showing which is the main block of content on this page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.

  2.Use relative units like rem or em instead of px to have a better performance when your page content resizes on different screens and devices. REM and EM does not just apply to font size, but all sizes as well. To save your time you can code your whole page using px and then in the end use a VsCode plugin called px to rem to do the automatic conversion or use this website https://pixelsconverter.com/px-to-rem

  3.Reduce the CSS you can use the direct selector for each element instead of using class this way you have a code even cleaner, for example, you can select everything using the direct selector for (img, h1, and p, main).

  ✌️ I hope this helps you and happy coding!

  Marked as helpful

  1
• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  Hi, David! 👋

  Congratulations on completing your first Frontend Mentor challenge! 🎉

  I recommend removing Bootstrap. It is overkilled to use a CSS framework to finish a small challenge. Also, it will slow you down in mastering CSS. So, I suggest writing all the styling without using any CSS framework.

  More suggestions:

  • Remove the top and left properties from the main-box and img elements. Those properties are not working with the static position. For your information, by default, all elements are position: static.
  • Also, remove the top, right, left properties from the h1. It's not needed.
  • There's no need to set width and height to any elements. I am guessing that you were copy-pasting the CSS code from Figma (or Sketch). Those code snippets aren't supposed to be used. You need to think about whether or not the code snippet is appropriate.
  • I notice that the site is importing unnecessary font weights (wght@100;200;300;400;500;600;700;800;900). Only import the necessary font weights.
  • I suggest making the img as a block element and setting max-width: 100% to make it easier to work with img element.
  • The card only needs a max-width to prevent it from filling the entire page. No need for width and height.

  Hope this helps! 🙂

  Marked as helpful

  0