Qr code card featuring Responsivnes and flexbox

Community feedback

• 

  Saul Gabriel Aranguren•320

  @saularangurenPosted 6 months ago

  Greetings, your solution is really impressive, however, I am going to help you with some tips so you can improve your web layout skills.

  Start by eliminating code blocks (grouped lines) that are unnecessary in your project:

  <!-- displays site properly based on user's device -->
  <!-- Feel free to remove these styles or customise in your own stylesheet 👍 -->
  
  <!-- Improve your front-end skills by building projects
  
    Scan the QR code to visit Frontend Mentor and take your coding skills to the next level
    
  <div class="attribution">
      Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. 
      Coded by <a href="#">Your Name Here</a>.
  </div> -->
  

  These lines of code, including the comments, are not playing any role.

  Let's continue with improving your semantic html, replace the <div class="qr-code-container"></div> tag with the <main class="qr-code-container"></main> tag and replace the <div class="text-cont"></div> tag with <section class="text-cont"></section>, this is how your project should look:

  <main class="qr-code-container">
      <img src="images/image-qr-code.png" alt="" class="qr-img">
      <section class="text-cont">
          <h3>Improve your front-end skills by building projects</h3>
          <p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
      </section>
  </main>
  

  Finally, you must eliminate the <div class="wrapper"> tag and the last </div>, it should not be equieta, that is what body is for because this will be as its name says the main body of your page, and don't forget to change the .wrapper class in the css to body, eliminate the width: 100%; css rule, it is not necessary and modify the height: 100vh; rule to min-height : 100vh;, by saying min-height you are telling the browser: it does not matter if my website is much longer than the user's screen, it will not be limited, otherwise it will be limited and will not let you scroll if the page is longer

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

  happy codign 😁

  Marked as helpful

  1

  ---

  

  SIDA•190

  @GSida015Posted 6 months ago

  @saularanguren thank you for your feedback,I appreciate it and will consider your advice in my future projects!💪🏻😉

  1
• 

  P

  DalaScript•530

  @DalaScriptPosted 6 months ago

  Hey, Good job!

  კარგი ნამუშევარია მომეწონა, ასევე ძალიან მომეწონა Github-ზე გაფორმება. გაუმჯობესებისთვის მოგცემ რამდენიმე რჩევას :

  1. HTML ფაილში, head თეგში style-ის ელემენტი აღარ გჭირდება, ის თვისებები რა არის style-ის თეგში არის attribution კონტეინერისთვის, რომელიც თემლეითს მოყვება იდეაში, მაგრამ ეს წაშლილი გაქვს და შესაბამისად style თეგი აღარ არის საჭირო.

  2. Font-ის დალინკვა დაგავიწყდა, 'outfit'-ის ფონტია და შეგიძლია ამ ლინკზე ნახო.

  3. გამოიყენე ისეთი ელემნტები, რომელიც არის სემანტიკურად გამართლებული, როგორიც არის main თეგი მაგალითად, div კონტეინერი რომელსაც აქვს wrapper-ის კლასი, მის მაგივრად შეგიძლია გამოიყენო main თეგი, და div კონტეინერი რომელსაც აქვს qr-code-container კლასი, მის მაგივრად შეგიძლია გამოიყენო article თეგი.

  4. ასევე მეტი ყურადღება მიაქციე დეტალებს, qr-code-ის ფოტოს border-radius-ზე მეტი გაქვს მითითებული ვიდრე უნდა იყოს.

  5. text-cont კლასის p ელემენტს არ აქვს text-shadow თვისება.

  6. qr-card-ს წაუშალე height-ის თვისება, რომ ზომი დაემთხვეს.

  7. თუ არ გაქვს პრემიუმი ნაყიდი Fronendmentor-ზე, მაშინ გირჩევ რომ, როცა ცვლილებებს შეიტან შენ პროექტში, ახალად ატვირთო, და არ გაუკეთო სქრინშოთის ახალი გენერირება, იმიტომ რომ შეზღუდული ხარ ახალი სქრინშოთების გენერირებაში.

  წარმატებები!

  Marked as helpful

  0
• 

  P

  Øystein Håberg•10,650

  @Islandstone89Posted 6 months ago

  HTML:

  • Every webpage needs a <main> that wraps all of the content, except for <header> and footer>. This is vital for accessibility, as it helps screen readers identify a page's "main" section. Wrap the card in a <main>.

  • The image has meaning, so it must have proper alt text. Write something short and descriptive, without including words like "image" or "photo". Screen readers start announcing images with "image", so an alt text of "image of qr code" would be read like this: "image, image of qr code". The alt text must also say where it leads(frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website."

  • Headings should always be in order, so you never start with a <h3>. Change it into a <h2>.

  CSS:

  • Including a CSS Reset at the top is good practice.

  • I like to add 1rem of padding on the body, to ensure the card doesn't touch the edges on small screens.

  • font-family should be placed on the body.

  • I would move the properties on .wrapper to the body .As mentioned above, you don't need the width ,and height must be min-height.

  • Remove ALL widths and heights in px. Setting fixed sizes is not recommended for the web, as we want our content to adapt to different screens.

  • Add a max-width of around 20rem on the card, to prevent it from getting too wide on larger screens.

  • Paragraphs have a default value of font-weight: 400, so there is no need to declare it.

  • Since all of the text should be centered, you only need to set text-align: center on the body, and remove it elsewhere. The children will inherit the value.

  • I would remove the text-shadow, as it makes the text harder to read.

  • You don't need to declare font-style: normal, as that is the default.

  • On the image, add display: block and max-width: 100% - the max-width prevents it from overflowing its container.

  • As the design doesn't change, there is no need for any media queries. When you do need them, they should be in rem, not px. Also, it is common practice to do mobile styles first and use media queries for larger screens.

  Marked as helpful

  0

  ---

  

  SIDA•190

  @GSida015Posted 6 months ago

  @Islandstone89 Thank you for your feedback,I will consider it...IDK if I will or not change this project but I will consider your advice in my future projects!!!🙂

  1