Pure CSS with flexbox

Community feedback

• 

  P

  Rupali•780

  @rupali317Posted 3 months ago

  Hi @junwei-wong

  Congratulations on completing your first challenge. I have the following feedback for your code (in terms of enhancing the accessibility and semantics of the HTML):

  • You should always include a CSS reset for your projects. Different browsers may have different default stylings for UI elements. In order to make the CSS stylings consistent across all browsers , you should reset the CSS. Please refer to the following blog on CSS reset
  • For font size you should always use the rem unit instead of pixel. Pixel is not good for scalability and thus it will lead to poor accessibility. For instance, I am a user who relies on large font. So I will go to my browser settings to alter the font settings to a larger one. I will expect that the font in your qr code site to scale accordingly. But since you have used pixels, the font will remain the same and it will not scale to a larger one.
  • Your HTML structure in relation to the <main> tag is currently as follows:

  <main>
    <img>
    <div>
      <label></label>
      <label></label>
    </div>
  </main>
  

  Semantic HTML is preferable since it enhances the accessibility of your page and it leads to better SEO. Your code can be more semantic as follows:

  <main>
    <img>
      <h1></h1>
      <p></p>
  </main>
  

  A heading is represented as <h1>, <h2>, <h3>, <h4>, <h5>, <h6> A paragraph is represented as <p>. They are not appropriate as <label>

  • In your CSS, you have created CSS variables like --slate-300, --slate-500 etc. You can also extend using CSS variables for other properties like colors, font size, font weight etc so that you can reuse them and maintain the changes easily. For example, if the color scheme changes from blue to green, rather than having to change in various places, you just change the value of the CSS variable. For example:

  :root {
  
      /* Colors */
      --background-color: hsl(212, 45%, 89%);
      --card-color: hsl(0, 0%, 100%);
      --link-color: hsl(228, 45%, 44%);
      --main-text-color: hsl(218,44%,22%);
      --sub-text-color: hsl(220, 15%, 55%);
  
      /* Typography */
      --font-size-main: 1.375rem; /* 22px */
      --font-size-sub: 0.9375rem; /* 15px */
      --font-size-footer: 0.8125rem; /* 13px */
      --font-weight-bold: 700;
      --font-weight-regular: 400;
      --line-height-normal: 1.2;  /* Normal */
  
      /* Spacing */
      --space-none: 0;
      --space-base: 1rem; /* 16px */
      --space-m: 1.5rem; /* 24px */
      --space-l: 2.5rem; /* 40px */ 
  
      /* Border radius */
      --card-border-radius: 20px;  
      --img-border-radius: 10px;
  
      /* Box shadow */
      --card-box-shadow: 0px 25px 25px 0px rgba(0, 0, 0, 0.05);
  
  }
  

  Let me know if the above suggestions help.

  Marked as helpful

  1
• 

  Lance•1,580

  @IzykGitPosted 4 months ago

  Good work!

  While you do need flexbox to center the card on the page you can do this with less flexbox by using no div containers! You can just have the image and the text in the card and use text-align: center to center everything in the middle of the card!

  Keep up the good work!

  Marked as helpful

  0