@GregLyons
Posted
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