Design comparison
Reports
Links must have discernible text
<a href="#" class="footer__brand">
<img src="assets/img/logo-white.svg" alt="" class="footer__brand--img">
</a>
Learn more All page content should be contained by landmarks
<div class="testimonial__top">
<h2 class="testimonial__top--secondheading">
What they’ve said
</h2>
</div>
Learn more All page content should be contained by landmarks
<div class="testimonial__middle--main swiper-wrapper" style="cursor: grab; transform: translate3d(-1300px, 0px, 0px); transition-duration: 0ms;" id="swiper-wrapper-5359d0c382cb10397" aria-live="off">
Learn more All page content should be contained by landmarks
<div class="testimonial__middle--pagination swiper-pagination swiper-pagination-clickable swiper-pagination-bullets swiper-pagination-horizontal">
Learn more All page content should be contained by landmarks
<div class="testimonial__bottom">
<a href="#" class="testimonial__bottom--cta">get started</a>
</div>
Learn more All page content should be contained by landmarks
<section class="career career--bg">
Learn more Luis’s questions for the community
Hi everyone.
I remade this challenge, but now using SCSS.
Feel free to give any feedback.
Happy coding :D.
Community feedback
@kenreibman
Posted
Hey @luis08201 ! Great submission. I love how you pushed yourself in using SCSS. Some suggestions from an industry standpoint:
-
Check your accessibility report that Frontend Mentor offers you, you have quite a few accessibility and HTML issues.
-
Your BEM naming is a little off. You're giving modifier classes too often; Usually to every third word in your element class name. the double hyphen
--
are for modifier events likebtn__large--active
orphoto__img--highlighted
in your case,header--nav
or--link
should be__link
ormain-header__nav
simply put, you should rarely have to use -- -
I see you're using
section
elements which is a great step into a well-structured page. However, you're lacking amain
element that surrounds thosesection
tags to make it semantically correct. You could wrap all your content inside thebody
tag with amain
element. -
I also suggest you put a nice descriptive alternative text
alt=""
whenever you use an image for accessibility purposes. Same goes for your anchor links as well. I see you're getting a lot of those errors in your footer. -
If your image is purely decoration and you absolutely believe that it doesn't need descriptive text for accessibility, you can hide that from screen readers by putting an
aria-hidden="true"
in yourimg
tag. You can read more about that here: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-hidden
I hope this helps, you're doing great!
Marked as helpful
Account Deleted
I think it looks very good!
Marked as helpful
Please log in to post a comment
Log in with GitHubJoin our Slack community
Join over 180,000 people taking the challenges, talking about their code, helping each other, and chatting about all things front-end!