Responsive design, fetch data from data.json

Community feedback

• 

  T

  Grace•30,990

  @grace-snowPosted over 1 year ago

  Hi

  This looks good but there are issues mainly with html you need to learn

  • section acts as a div when unlabelled. So you have content outside of landmarks here. I recommend wrapping all the challenge content in main, then attribution in footer as you have done. You can have sections inside main. There is no reason at all for the wrapping div around everything when you can style the body

  2. Content should never be in divs alone. This appears especially important in your generated js markup which is all divs with no meaningful elements currently
  3. The icons should have empty alt. They are decorative because the label is present in text already
  4. Display none is NOT sr only! That hides content completely from assistive technology
  5. I can’t work out what the min widths are there for in css. That’s a code smell I’d be investigating if on my computer (I’m on mobile atm). It’s very rare to need min width and I definitely wouldn’t expect it to have magic number values like 87%. It’s likely the min width 16 rem could cause breakage when users alter their base font size. Similarly it’s really unusual to see so many widths being set (eg in the media query). I’d expect a max width on the component, a width and height on the icons… and that’s probably it when it comes to width properties. The grid templates should handle the rest
  6. This snippet doesn’t make sense as you’re defining only one column in the template but 2 columns in the areas:

  grid-template-columns: 2fr;
      grid-template-areas:
          'main evaluation';
  

  7. Not wrong but it’s less performant to import fonts in css than link in the html head
  8. I think you may be misunderstanding how and why to use let in js. I’d expect to see const in most declarations there but it’s let almost everywhere at the moment. Worth reading about again and refactoring that

  Marked as helpful

  1