QR code component

Community feedback

• 

  Fer•3,970

  @fernandolapazPosted over 1 year ago

  Hi 👋, perhaps some of this may interest you:

  HTML 🧱, ACCESSIBILITY ⚖:

  🔹Semantic elements:

  • The main content of every document (the whole card in this case) should be wrapped with the <main> tag, just leave the attribution out.
  • There is no reason to skip headings, and every page should have an <h1>.

  🔹This is a meaningful image and therefore should have an alt text with a description in case the user cannot see it for some reason.

  CSS 🎨:

  🔹The card must have a "max width" that matches the layout provided for the desktop version. And the height should be determined by the content, without the need to define it.

  🔹The images provided are already optimized, it is not necessary to assign them measurements.

  🔹Also, as part of the CSS Reset, the following is very useful regarding images:

  picture,
  img,
  svg {
    display: block;
    max-width: 100%;
  }
  

  🔹Length units such as pixels may not be the best alternative because screen sizes and user preferences vary, and absolute units don’t scale. Relative units like em or rem are a better option for scalable layouts (the page will adjust to the user's browser settings) and maintenance (to make changes without having to adjust every pixel value).

  And since you asked about this topic, I leave this in case you want to take a look at it: The Surprising Truth About Pixels and Accessibility 🔎

  Please let me know if you disagree with something or if you would like more information on any of these topics.

  If you have any questions I’m here to answer so don't hesitate 🙂

  Regards,

  Marked as helpful

  1

  ---

  

  EvtimEvtimov•30

  @evtimov-ptrPosted over 1 year ago

  @fernandolapaz

  Hello Fer,

  Appreciate all of the valuable advices 🙂I will check the topic regarding the pixels. Also I just checked your youtube channel and saw the QR code component solution, loving the fact that you are using vars for the colors, I might also start doing it that way as it looks cleaner in my opinion. Also amazing solution overall 🙂

  0

  

  Fer•3,970

  @fernandolapazPosted over 1 year ago

  @evtimov-ptr

  Hi,

  You are welcome 🙂

  And thank you so much! 🙏

  0
• 

  Panji•2,090

  @pperdanaPosted over 1 year ago

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

  • I have some additional recommendations for your code that I think you'll find interesting and valuable.

  📌 Add <main> tag as semantic HTML in code

  • The <main> tag is a semantic HTML element that is used to define the main content of a web page.

  • The <main> tag should be used to wrap the primary content of a web page, such as the main article, section, or body of text.

  for example code:

  <main>
    <div class='container'>
      <h1>Article Title</h1>
      <p>Article content goes here...</p>
      .......................
    </div>
  </main>
  

  In the example above, the <main> tag is used to wrap the <div> tag, which contains the primary content of the web page. This tells both human readers and search engines that the content inside the <main> tag is the most important and relevant content on the page.

  I hope you found this helpful!

  Happy coding🤖

  Marked as helpful

  1

  ---

  

  EvtimEvtimov•30

  @evtimov-ptrPosted over 1 year ago

  @Panji200

  Тhank you Panji! Definitely need to work more on the semantic part!

  Kind Regards, Evtim

  0
• 

  Abdul Khaliq 🚀•72,560

  @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.

  ---

  CSS 🎨:

  • Looks like the component has not been centered properly. So let me explain, How you can easily center the component without using margin or padding.

  ---

  • We don't need to use margin and padding to center the component both horizontally & vertically. Because using margin or padding will not dynamical centers our component at all states

  ---

  • To properly center the component in the page, you should use Flexbox or Grid layout. You can read more about centering in CSS here 📚.

  ---

  • For this demonstration we use css Grid to center the component.

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

  • Now remove these styles, after removing you can able to see the changes

  .container {
    margin: 10rem auto;
  }
  

  ---

  ---

  • Now your component has been properly centered

  ---

  .

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

  Happy coding!

  Marked as helpful

  1

  ---

  

  EvtimEvtimov•30

  @evtimov-ptrPosted over 1 year ago

  @0xAbdulKhalid

  Hello Abdul,

  Many thanks for absolutely everything! Definitely taking notes on what you have suggested :) I wish you to have an amazing day Abdul!

  Kind Regards, Evtim

  0
• 

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

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

  If you want that this solution is responsive, I recommend some techniques without using media query for this solution

  • I think Flexbox is better than Grid in centering both horizontally and vertically. I recommend this method, it's up to you whether to apply it or not :)
  • If you want to make the card centered both horizontally and vertically, you'd better add flexbox and min-height: 100vh to the body

  body {
    /* display: grid; */
    /* place-items: center; */
    display: flex;
    flex-direction: column;
    align-items: center;
    justify-content: center;
  }
  

  • If you use max-width, the card will be responsive and you can increase the width a bit
  • You'd better give padding to give a gap between the content and the border of the card

  .container__content {
    /* display: grid; */
    /* place-items: center; */
    padding: 15px;
    background-color: #ffffff;
    border-radius: 12px;
    box-shadow: 0 0 5px #ccc;
    /* width: 280px; */
    max-width: 300px;
  }
  

  • 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

  .container__content img {
    /* width: 250px; */
    /* height: 250px; */
    width: 100%;
    border-radius: 12px;
  }
  

  • Finally, the solution will be responsive if you follow the steps above. You don't need .container anymore and you can remove it.

  /* .container {
    background-color: #ffffff;
    border-radius: 12px;
    box-shadow: 0 0 5px #ccc;
    width: 280px;
    height: 430px;
  } */
  

  Hope I am helpful. :)

  0