QR-Code-Component Challenge

Community feedback

• 

  Angela Moore•160

  @Cyber-ChicPosted over 1 year ago

  Your solution looks awesome! Great job completing your first challenge! 😊

  I have a one more recommendation that may improve your site:

  • Add a <h1> element to your code. All sites should include a minimum of one <h1> element to indicate the the most important (or highest-level) heading on a page.

  I hope this is helpful! Happy Coding!

  -Angie

  1

  ---

  

  Dominic Ewe•10

  @sneakynickyPosted over 1 year ago

  @Cyber-Chic Thank you! I'll take note on that and include it in my next challenge! :)

  0
• 

  Abdul Khaliq 🚀•72,580

  @0xabdulkhaliqPosted over 1 year ago

  Hello there 👋. Congratulations on successfully completing the challenge! 🎉

  • I have other recommendations regarding your code that I believe will be of great interest to you.

  ---

  HTML 🏷️:

  • This solution may cause accessibility errors due to lack of semantic markup, which causes lacking of landmark for a webpage and allows accessibility issues to screen readers, due to accessibility errors our website may not reach its intended audience, face legal consequences, and have poor search engine rankings, highlighting the importance of ensuring accessibility and avoiding errors.

  ---

  • What is meant by landmark ?, They used to define major sections of your page instead of relying on generic elements like <div> or <span>. They are use to provide a more precise detail of the structure of our webpage to the browser or screen readers

  ---

  • For example:
    • The <main> element should include all content directly related to the page's main idea, so there should only be one per page
    • The <footer> typically contains information about the author of the section, copyright data or links to related documents.

  ---

  • So resolve the issue by replacing the <div class="container"> element with the proper semantic element <main> along with <div class="attribution"> into a <footer> element in your index.html file to improve accessibility and organization of your page

  ---

  .

  I hope you find this helpful 😄 Above all, the solution you submitted is great !

  Happy coding!

  1

  ---

  

  Dominic Ewe•10

  @sneakynickyPosted over 1 year ago

  @0xAbdulKhalid Thanks for the helpful tips! I'll take note of these and include it in the next challenge.

  0

  

  Abdul Khaliq 🚀•72,580

  @0xabdulkhaliqPosted over 1 year ago

  @sneakynicky Glad you found it helpful ! 🤠

  0
• 

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

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

  • If you use max-width, the card will be responsive and you can increase the width a bit
  • You'd better update padding to give a gap between the content and the border of the card
  • You don't need to use flexbox here. If the styles don't work, I recommend you don't use them

   .card {
    /* width: 220px; */
    /* height: 350px; */
    max-width: 300px;
    /* padding: .5em; */
    padding: 15px;
    /* display: flex; */
    /* flex-direction: column; */
  }
  

  • 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 {
     width: 100%;
     /* width: 200px; */
     /* height: 200px; */
     border-radius: 10px;
     /* margin-top: .25rem; */
  }
  

  • You'd better update and add font-size and margin for the texts

  p.header {
    font-size: 20px;
    margin: 10px 0;
  }
  

  p.content-text {
    /* font-size: 12px; */
    font-size: 16px;
    margin-bottom: 20px;
  }
  

  • You don't need to use .card--title and .card--text and you can remove them

  /* .card--title {
    padding: .5em;
  } */
  
  /* .card--text {
    padding: 0 .5em;
  } */
  

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

  Hope I am helpful. :)

  1

  ---

  

  Dominic Ewe•10

  @sneakynickyPosted over 1 year ago

  @ecemgo Thanks! I wasn't really thinking about making it responsive but it definitely has to be something to have at the back of the mind in the future.

  I appreciate the code samples as suggestive updates too!

  1
• 

  skopelosbill•40

  @skopelosbillPosted over 1 year ago

  Now I have managed to visit your GitHub repo (yes is was my problem rather than yours) I see that your image size is 200px x 200px and your container is 220px x 325px. The brief is for the container to have a width of 375 pix which explains why yours is so small. I would suggest fixing this in the first place and then having a look at what the changes do.

  0
• 

  skopelosbill•40

  @skopelosbillPosted over 1 year ago

  Hi, your solution is rather small compared to what it should be but you have positioned it well on the page. I recommend that you search on you tube for Kevin Powell. Kevin has a huge amount of videos on every possible aspect of CSS and I subscribe to his channel, there is so much on there that is so helpful. I can’t comment on your html because I can’t see it but that may be my problem rather than yours. You have made a good start to your front end Mentor journey.

  0