3-column-preview, Html, Css, Responsive Layout

Community feedback

• 

  Fluffy Kas•7,675

  @FluffyKas

  Posted over 3 years ago

  Hey there,

  Your solution looks really good! All screen sizes seem to work well, I'd just like to discuss a few bits of your code.

  1. The height on your "main__container": try removing it and see what happens. :) Not much, right? Setting height is usually not necessary (there are exceptions, of course). Your elements have their own default height which you can increase by giving them a padding.

  2. Setting 200vh height on the body might be a bit too much? A min-height: 100vh is usually enough (plus some margin-top and -bottom if you'd like).

  3. You use width and max-width and you give them the same values which doesn't make a lot of sense. width: 400px will set a fixed width on your component so max-width then don't do much for responsiveness if given the same value. What you could do instead is something like this: width: 90%, max-width: 400px. Or, if you'd like to make it even shorter: width: min(90%, 400px). Adding min means the smaller of the 2 values will be chosen and applied to your component automatically so it's exactly the same as the previous example, just in one line. Try this out, see the difference it makes :)

  4. You should leave alt texts empty in this challenge, as they're really just decorative. You could give them an aria-hidden= true to make sure the screen reader skips them.

  I hope these tips will help you a bit in the future. All in all, your solution is still great! ^^

  01

  Marked as helpful
• 

  Alex Nix•170

  @AlexNixx

  Posted over 3 years ago

  Hi! Great job, i really like it. After code review i wanna to say, if u have 3 same column use one class to compose it, and add a second class to style it.

  03