Single price grid component (newbie)

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  👋 Hi Vincent!

  🎉 Congratulations on finishing your first challenge! I have some feedback that will improve this solution:

  • Accessibility
    • You can directly style the <a href="#" class="btn-link">Sign Up</a> to make it look like a button (no div needed).
    • Create a custom :focus-visible styling to any interactive elements (button, links, input, textarea). This will make the users can navigate this website using keyboard (Tab) easily.
    • This text content should not be a heading. The below markup would be a better solution and also you can make all h3 to h2.

  <p class="heading_secondary">
    <em>30-day, hassle-free money back guarantee</em>
  </p>
  

  • The list items on Why Us section should not be links. It's just a list of reasons why choosing that platform 🙂.
  • I would recommend to not changing the root (html) font size. It can cause a lot of issues. Read what an accessibility expert (Grace Snow) has said about this.
  • Responsiveness
    • I would recommend setting max-width with rem unit instead of setting width for each screen size.
  • Best Practice
    • line-height should always be unitless and you can set the main line-height value on the body element. All elements will inherit the line height from the body element.

  body {
    line-height: 1.6;
  }
  

  • I would recommend treating the body element as your main element, not html element. All browsers treat body element as the main element on the page, not html.

  /**
   * 1. Use rem unit for body font size so that all users
   *     can change the font size based on their needs.
   */
  body {
    font-size: 1rem; /* 1 */
    background-color: rgb(225, 235, 244);
    font-family: "Source Sans Pro", sans-serif;
    font-weight: 400;
  }
  

  • Tips
    • For the next challenge if you are using Sass, I would recommend creating a function that will convert all px to rem unit.

  // How it works
  
  /// On Sass file (I am using SCSS syntax)
  body {
    font-size: rem(16);
  }
  
  /// On CSS
  body {
    font-size: 1rem;
  }
  

  That's it! Hopefully, this is helpful!

  Marked as helpful

  1

  ---

  

  Vincent Chen•120

  @VincentChen6345Posted almost 3 years ago

  @vanzasetia Wow thank you so much for the thorough advice!! Guess there's a lot of 'best practices' that I haven't learned yet.

  0

  

  Vincent Chen•120

  @VincentChen6345Posted almost 3 years ago

  @vanzasetia I've implemented all the changes except for html {font-size:62.5%} .

  https://github.com/VincentChen6345/single-price-grid-component

  I'll keep it at the default value for the next project. My udemy teacher taught me the 62.5% method which you guys seem to disagree with. I did some googling and It seems like theres a debate between whether to keep it at default or use 62.5% so that 1rem=10px.

  0