Responsive QR Code using height and width Only CSS only

Community feedback

• 

  Adriano•38,390

  @AdrianoEscarabotePosted almost 2 years ago

  Hello Muhammad Talha Rehman, how are you? I truly loved your project's outcome, however I have some advice that I hope you'll find useful:

  A good practice to center content is using grid or flex-box, avoid using margin or padding to make placements, use only in the latter case! we can do it like this:

  Flex-box:

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

  GRID

  body {
      display: grid;
      min height: 100vh;
      place-content: center;
  }
  

  You could have used a h1 to make the project more accessible. Every page must have a level 1 header that identifies the primary title and follows the h1-h5 format for screen readers.

  <h1 class="heading">Improve your front-end skills by building projects</h1>

  The remainder is excellent.

  I hope it's useful. 👍

  Marked as helpful

  1

  ---

  

  Muhammad Talha Rehman•10

  @talhawebguruPosted almost 2 years ago

  @AdrianoEscarabote Thank You So much. I appreciate your advice. It's really helpful. Thanks for giving me advice. Please also advise on my forward project. From onward, I will use flex to center items. Thanks a lot, brother.

  1

  

  Adriano•38,390

  @AdrianoEscarabotePosted almost 2 years ago

  @mtalha987 no problem, happy coding!!

  0
• 

  Ahmed Hany•220

  @ahmedhanyhPosted almost 2 years ago

  Hey @mtalha987! Congratulations on completing your project! You've done a great job!

  To answer your question about minimizing the code:

  In HTML

  you don't need to remove anything.

  In CSS

  You can minimize your CSS by doing the following: Move the repeated declarations in the .qrCode, .heading, and .detail rulesets to a new ruleset with a selector that groups them together by separating them with a comma (a grouping selector), like so:

  .qrCode, .heading, .detail {
    margin: 0 auto;
  }
  
  .heading, .detail {
    padding-top: 20px;
    text-align: center;
  }
  

  or using the functional pseudo-class selector :is() like so:

  :is(.qrCode, .heading, .detail) {
    margin: 0 auto;
  }
  
  :is(.heading, .detail) {
    padding-top: 20px;
    text-align: center;
  }
  

  We can also move the text-align: center; declaration to the body rule (and also remove it from .attribution):

  body {
      ...
      text-align: center;
  }
  

  There are also lots of unnecessary whitespaces across the stylesheet that we can get rid of.

  There are still some improvements that can be made

  In HTML

  Use semantic HTML elements instead of <div>s to improve your website accessibility. Use <main> and <footer> landmarks, and the <h1> heading in your HTML like this:

  <main class="container">
  ...
    <h1 class="heading">
      ...
    </h1>
  </main>
  
  <footer class="attribution">
  ...
  </footer>
  

  Some useful resources to learn more about semantic HTML and how to use them to structure your content:

  • web.dev's content structure module
  • The Odin Project's semantic HTML lesson

  In CSS

  Use max-width instead of width for .container to allow its size to shrink on smaller screen sizes:

  .container {
      ...
      max-width: 300px;
      ...
  }
  

  In general, don't use width and height, use min-width, max-width, min-height, and max-height instead for responsiveness.

  I hope the feedback was useful. I wish you the best with your journey on FEM.

  0

  ---

  

  Muhammad Talha Rehman•10

  @talhawebguruPosted almost 2 years ago

  @ahmedhanyh Thanks bro. Your advice is also appreciatable. From onward, I will follow your advice. Width and height advice is really useful, and semantic HTML is also. Thanks for giving me advice . Please also give me advice on my next project.

  1

  

  Ahmed Hany•220

  @ahmedhanyhPosted almost 2 years ago

  @mtalha987 I'm glad I've helped! Keep up the good work!

  0
• 

  CLRwebs•60

  @CLRwebsPosted almost 2 years ago

  Hey, Your solution looks pretty good. You asked about the "code minimizing" , you have some rows what is unnecessary. For example on .qrCode you wrote the margin twice, you can also delete the empty rows on your HTML and CSS file. I don't really use 'px' for margin and padding , but It's really up to you. Other then that you made a good solution. Keep up the good work!

  0

  ---

  

  Muhammad Talha Rehman•10

  @talhawebguruPosted almost 2 years ago

  @CLRwebs Thanks brother.

  0