. 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.
Francis Wafula
@franco2keAll comments
- @ibra-sanSubmitted over 2 years ago@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 helpful0 - @EmmanuelOlokeSubmitted almost 3 years ago
There's always room for improvement. I would love feedback on how I can make the project better.
@franco2kePosted almost 3 years agoHi 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 helpful0 - @samudriawanSubmitted almost 3 years ago
any suggestions are welcome, thank you
@franco2kePosted almost 3 years agoHi 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 - @ganeshaa59Submitted almost 3 years ago
Any Feedback most wekcome!
@franco2kePosted almost 3 years agoHi @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