HTML CSS and JS

Community feedback

• 

  P

  Matt Pahuta•580

  @MattPahutaPosted 7 months ago

  Hi there. Nicely done completing this challenge. You're using semantic HTML landmarks well here and matched the design comp quite closely.

  Here's some specific feedback and tips to carry forward to future projects:

  1. Always use a modern css reset at the start of the styles in every project. Andy Bell and Josh Comeau both have excellent resets worth checking out.

  2. You should use min-height instead of height on your body element. There's no effect for this particular project, but the browser should know it's allowed to grow beyond the viewport height when necessary. Width should not be set on the body. By default, it's 100% and shouldn't be changed.

  3. Likewise, the width and height properties of your main element are not necessary. You could remove all these and put a display of grid on your body to produce similar results.

  4. Media queries should be set in rem/em, not px. Here's a great resource

  5. For your social share component, you're setting base styles using an id selector (#socials) but toggling hidden/show styles using a class (.hidden). I think this is causing some specificity issues and impacting how the card is displaying on different screen sizes in different states. In general, it's best practice to avoid mixing IDs and classes to apply styles.

  6. It's bad practice and an accessibility concern to set a font size on the html element. Instead, set the base font you want on the body or a lower-level element.

  7. Your div with an ID of share-div-2 doesn't appear to be necessary. I think you can omit this markup and the related JS code.

  Good luck with future challenges!

  0