Responsive HTML/CSS Using Grid

Community feedback

• 

  P

  Fluffy Kas•7,715

  @FluffyKasPosted about 3 years ago

  Hey there,

  Your solution seems pretty good, your grid works nicely ^^ Since you've got the worst bit out of the way, I'm going to give some feedback on the rest.

  1. The background color you chose isn't the right one. Not a big issue but since we're aiming to get as close to the original design as possible I thought I mention it ^^

  2. Semantic bits: container div should be swapped for main. Also, HTML headings have a semantic meaning, they shouldn't be chosen based on what default styling they have. You should aim to have an <h1> for the main heading of your page and decrease it from there, one at a time. For example, my approach would be: <h1> for "Join our community", <h2> for "Monthly subscription" and "Why Us", <h3>, and then <h4> if there's anything else (I'm not sure there is). There's a new video from Kevin Powell that shows really well, why these things are important ^^ The "Why Us" section has a more semantic approach too. You used a div with some ps inside but actually it's a list. ^^ I'd go with a <ul> with <li>s inside.

  3. Class names: choosing the right names will be important in bigger projects. I'd suggest choosing names that are descriptive about the content and not the style (although this is fine to do so sometimes, but shouldn't be the main point when choosing class names). You (and your team mates) will thank yourself later, as choosing the right names will make it much easier to navigate through your code.

  4. Your button uses the default sans serif at the moment, instead of your chosen font-family. Buttons have a lot of default styling that you need overwrite I'm afraid.

  5. I'd make the font-sizes just a tiny bit bigger, to make things easier to read!

  Good luck :)

  0