qr code challenger

Community feedback

• 

  RabicaTahir•500

  @RabicaTahirPosted over 1 year ago

  salam o Alaikum! @Diogopegorel. 🎯Congratulations on completing the challenge !

  💬I have some suggestions about your code.

  HTML 🔔:

  ▪ It is advisable that you place all your main site content in a landmark element <main>. Wrap the body code in main tag. Click & Wrap the code

  <div class="container">

  ▪ Best practice is to ensure that the beginning of a page's main content starts with a h1 element. Replace this

  <p class="p1">Improve your front-end <br>skills by building projects</p>

  ▪ Instead of using the <br> tag , you should use a semantic HTML element and the CSS margin or padding properties.

  CSS: 🎨

  📍▪ Check the style guide & change the background color.

  📍▪ Instead of using pixels in font-size, using relative units like em or rem is a good practice.

  I hope you find it useful! By the way, the solution was great. 🙌💯

  Enjoy coding, Stay Safe! 🤞

  Marked as helpful

  1

  ---

  

  Diogo Pegorel•10

  @DiogopegorelPosted over 1 year ago

  Good night@RabicaTahir

  Thank you for your helpfull advices.

  I am starting my front end learning yet and all your tips are always welcome.

  I´ve already changed my code in order to improve it and I´ve applied your tips.

  Enjoy coding, stay safe as well 😀

  1
• 

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

  Some recommendations regarding your code that could be of interest to you.

  • If you want to make the card centered both horizontally and vertically, you'd better add flexbox and min-height: 100vh to the body
  • For the color of the screen, you can use the recommended color in the body

  body {
    /* background-color: lightgray; */
    background-color: hsl(212, 45%, 89%);
    display: flex;
    flex-direction: column;
    justify-content: center;
    align-items: center;
    min-height: 100vh;
  }
  

  • When you use flexbox in the body, you don't need to use margin in the .main to center the card
  • You can reduce the width a bit
  • You'd better add padding to give a gap between the content and the border of the card

  .main {
    /* max-width: 350px; */
    max-width: 300px;
    /* margin: auto; */
    /* margin-top: 10%; */
    padding: 20px;
  }
  

  • In addition to that above, in order to make the card responsive and the image positioned completely on the card, you'd better add width: 100% to the img

  img {
    /* margin-top: 20px; */
    /* height: 300px; */
    border-radius: 5%;
    width: 100%;
  }
  

  • You'd better remove padding from the p

  p {
    /* padding: 0 20px 30px 20px; */
  }
  

  • You can update footer in this way

  footer {
    margin-top: 5px;
    /* margin-top: 50px; */
    /* padding: 8px; */
  }
  

  • Finally, if you follow the steps above, the solution will be responsive.

  Hope I am helpful. :)

  Marked as helpful

  0

  ---

  

  Diogo Pegorel•10

  @DiogopegorelPosted over 1 year ago

  Thank you @ecemgo

  The flexbox workwd quite well.

  I have changed my code the way you told me and it´s much better now.

  I apreciate that.

  1

  

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

  Happy to help, @Diogopegorel :)

  0