Skip to content
OverviewSolutions
1
Comments
2
Adam Turner
@adamjamesturner93

All comments

  • Adam Turner•55
    @adamjamesturner93Posted over 3 years ago

    Hey,

    Congrats on your first challenge!

    Your looks nice and clean which is great, very easy to read and follow. Your media queries are nice and clear too, especially like the fact you abstracted them away for cleanliness! Great to see using CSS Grid too - a really nice solution :)

    A few areas to improve: mostly around Semantic and accessible markup:

    • Images: Whenever you have images, always provide a valid alt tag - this is critical for users using screen readers.
    • Titles: Only have a single h1 rendered on a page, this is read by screenreaders as the Page Title, and so having multiple leads to confusion and a lack of clarity. Use CSS to create styles, HTML to provide semantic markup.
    • divs: you have a lot of divs where they really aren't needed. I try to consider them a last resort! For example, maybe removing the <div class="container">, and a section for each column, and replace the inner div with an article.

    They're small details, and things that can easily be solved. So genuinely - a great first challenge! 🤩

    2
  • Adam Turner•55
    @adamjamesturner93Posted over 4 years ago

    Source code looks good on the whole and is laid out nicely.

    • Nice use of variables and mixins in your scss

    Points to work on:

    • Try to avoid so many divs and use more semantic markup. Where you have <section class="content-container"> consider making this <main ...> then each <div class="content-container-items"> can become a <section>.

    • <li> elements must be child elements on <ul> tags.

    • Make sure to provide helpful 'alt' tags on images. These are vital for people using screen readers or on low bandwidth connections to get a sense of the image they are unable to see.

    1