Responsive Landing Page using Sass

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted almost 2 years ago

  Hi

  Sections are not actually landmarks unless labelled, and 99% of the time they're not needed. They act like divs. It's fine to use them but these should all be inside a main

  I recommend you remove your .wrapper div altogether. You shouldn't ever use huge paddings like that. It is a better practice to have a wrapper class that sets a max-width for the content, a little padding on the sides and a margin auto on the sides. You then use that class wherever it is needed lower down the DOM. At the moment the pseudo elements (e.g. on top-section) are causing horizontal overflow. That's where instead you'd let that section be 100% wide (edge to edge) but set it's overflow to hidden, then use the wrapper class inside for the main content to be within that wrapper.

  You are misusing aria-labelledby on the logo. It's pointing to an ID that doesn't exist. It doesn't need this attribute at all as you've already got alt on the image

  Set vertical margins on text elements. At some screen sizes text elements are touching each other

  Alt does not need to ever say words like "image" or "picture". They are already an image role. Instead they should describe what the image actually looks like

  The images in the project-description section need object-fit: cover; height: 100%;

  The icons in product-percs are decorative, so alt should be blank. You should also give these icons a fixed height and width.

  Marked as helpful

  1
• 

  P

  Allan Martinez•200

  @allmtzPosted almost 2 years ago

  Hey not sure what the best approach is but here are a couple different ideas for the 'filter' issue

  1. Set the height of the phone and keyboard image to 100% or
  2. Wrap the image in a <div> and then overflow:hidden the <div> or
  3. Add grid rows and span the image across the rows so it only takes up the space it needs

  Let me know if any of these work !

  Marked as helpful

  0