html css

Community feedback

• 

  

  Grace•32,130

  @grace-snow

  Posted over 4 years ago

  Hi Nemanja,

  I think this looks pretty good so far, it's definitely on the way so don't be too harsh on yourself.

  I recommend you

  • try to use smaller margins and only use padding where it's needed
  • let layout grow and shrink by using flex- properties like grow, shrink and basis
  • don't overcomplicate by wrapping every line of h1 in a span, that's not necessary. Maybe give the h1 a max-width in ch units instead?
  • Remember every bit of text needs to be in a semantic element, not just a span or div
  • In alt text, you don't need to use words like 'image' because they are already announced as images.
  • Make use of the gap property to separate flex items (won't work on old browsers though, it's pretty new)
  • I always recommend against changing your html font size down to 62.5%. People do that because they think they need to always know what 1 rem is and force rems to be round numbers. It's just not necessary and can cause lots of accessibility issues.
  • If you use negative margins on elements (perfectly fine way to acheive the layout btw!) make sure you prevent overlap by giving the parent element the same amount of padding on that side (eg padding-left: -15rem on rating-container would prevent overlap)

  The biggest things that I think would help you though is starting from mobile-first. Start with the simplest layout, view it in browser with devtools open at the side. As you start to widen the viewport and think there's room to move to the desktop design, that's when you introduce your first min-width media query.

  If you take this approach, I think your css will be shorter and you'll identify layout issues sooner.

  All the best with it. Maybe try the Profile Card or Single Price Grid component if you're not feeling that confident yet. Those are both great challenges for flexbox and CSS grid, respectively 😃

  11