HTML5 CSS3

Community feedback

• 

  P

  Grog the Frog•480

  @GregLyonsPosted over 2 years ago

  I like the use of flex in your media query; that's something I need to work on more. Here are a couple suggestions I have:

  1) You do a pretty good job of using semantic HTML, but I think in your "Why Us" section you should be using a <ul> instead of a series of <p>'s. While stylistically it looks the same, it's important for accessibility. Specifically, when you use <ul> with <li>'s, screen readers will indicate to the user that they've encountered a list, and it will even tell them the number of items. Whereas I can tell visually from your design that the features are in a list, if I were only having the content read to me I might be confused since I would just hear a series of incomplete sentences; it might take a few seconds before I realize I'm being told a list.

  In this particular case you'd need to remove some styling from the <ul> and the <li>'s (I always need to look up the rules for that...), but I believe that's also good practice since it helps you learn more about styling lists.

  2) This may be more of an opinion I have, but I think some of your CSS selectors are unnecessarily specific, e.g. .col_1 .money_back, .col_2 .sub_plan, .col_2 .price. On the one hand, it can prevent the styling there from leaking out to elements in different contexts using those classes. On the other hand, it makes refactoring harder since if you were ever to change the .col_1 or .col_2 class names, or to move the .price, .money_back, or .sub_plan elements out from the columns, then those rules would no longer apply. Thus, it may be better to use the second part of your selectors (i.e. without the column part) in each rule. I found that using BEM naming conventions was helpful, but other CSS naming conventions (like CUBE CSS) also help alleviate this issue.

  Hope this helps!

  Marked as helpful

  0
• 

  Ivan•2,630

  @isprutfromuaPosted over 2 years ago

  Hi there. You did a good job 😎

  keep improving your programming skills🛠️

  your solution looks great, however, if you want to improve it, you can fix these points:

  ✅ you must follow the order of the headings after the heading of level 1 should be followed by headings of level 2, not 3 as you have

  ✅ don't do this please. it makes no sense to reduce the font of the document and then redefine it to 16 pixels. It is better to use relative units. While modern browsers can smoothly zoom pixel-based layouts, sizing type in relative units ensures an entire layout can be scaled up or down by simply updating the font-size of the body element.

  
  html {
      font-size: 62.5%;
  }
  
  body {
      background-color: var(--light-gray);
      font-family: 'Karla', sans-serif;
      font-size: 16px;
      font-size: 1.6rem;
      height: 100vh;
  }
  

  ✅ You Need to Stop Targeting Tags in CSS. When you add CSS directly on tags, your markup can’t change. Your style is tightly coupled to your DOM, and any change increases the risk of breaking things.

  .col_3 h3
  

  I hope my feedback will be helpful. You can mark it as useful if so 👍

  Good luck and fun coding 🤝⌨️

  Marked as helpful

  0