Skip to content
OverviewSolutions
4
Comments
4
Francis Wafula
@franco2ke

All comments

  • Ibrahim San•230
    @ibra-sanSubmitted over 2 years ago

    . The area I am unsure of is the compatibility with different screen sizes. Also not sure how professional is my code and if I have applied good coding practices or not.

    Product preview card component

    1
    Francis Wafula•80
    @franco2kePosted over 2 years ago

    Hi, Just had a look at your project. Great work, I like your classnames, you got BEM methodology on lock. Two suggestions;

    • For the card details container 'card__perfume--details' setting justify-content to 'space-between' would have been easier than applying margins to all the child elements in my opinion. Not adding custom margins also helps with resuability in other future projects.
    • See the frontend mentor 'accessibility' report for additional tips if you haven't. Otherwise, good stuff.
    Marked as helpful
    0
  • Francis Wafula•80
    @franco2kePosted about 3 years ago

    Hi Emmanuel, congratulations on clearing this project. I just noticed that at exactly the 768px breakpoint, the why us container leaves some whitespace to the right, which disappears on resizing to 767. You might want to look into that.

    Marked as helpful
    0
  • Francis Wafula•80
    @franco2kePosted about 3 years ago

    Hi Dyota, congratulations on submitting your project. Please go through the provided report and make corrections on your submission. You will probably learn a lot from that.

    1
  • Francis Wafula•80
    @franco2kePosted about 3 years ago

    Hi @ganeshaa59 congratulations on clearing your project. Have you noticed the accessibility issue you are having? This will make it difficult for users utilising assistive technologies, e.g blind users to navigate your site. It will also lower your SEO ranking.

    It is caused by having a <h3> heading immediately following a <h1>. You need to turn the <h3> header to a <h2> and style it independently. Alternatively to improve your SEO as well as accessibility consider wrapping both headings in a <h1> element as below:

    <h1 class="heading-primary"> <span class="heading-primary--main">Join our community</span> <span class="heading-primary--sub">30-day, hassle-free money back guarantee</span></h1>

    Then you can style 'heading-primary--main' and 'heading-primary--sub'. This will allow your grid component to have a simple heading hierarchy and boost your SEO.

    Also please go through your accessibility report.

    0