Design comparison
Solution retrospective
Is my use of BEM classes in this solution could be considered a Best practice? Because I struggled a lot trying to decide which would be the Block and the Element.
and of course, any feedback and suggestions on how I can improve are very welcome!
and Happy Holidays :).
Community feedback
- @pikapikamartPosted almost 3 years ago
Hey, glad to see you submit another solution. The desktop layout looks really great, it is responsive and the mobile state looks really great as well.
For the BEM usage on this, I think it looks fine on this one. For this, you don't need a general block element that could be applied to certain section or element since the layout is not that big so the specific usage on this is actually fine.
Here are some other suggestions for the site:
- For each of the review-list-item, you could use the stars as value for the
li
background
property. This way you don't need to create extraimg
tag on the markup. If you somehow use it asbackground
then you can just remove as well thespan
nesting the review text since you can useli
directly as text-container and less some selector. - For each of the person's
img
, remove the wordpicture
on it. As I mentioned before, words that relates tographic
should be avoided when using thealt
sinceimg
is already an image/picture. - For each testimonial card, you can use this markup below:
<figure> <img src="" alt={person name}> <blockquote> <p> {qoute in here}</p> </blockquote> <figcaption> person name <p> person role </p> </figcaption> </figure>
Using
figure
withblockquote
can make the whole content be read as single by screen-reader. For this though, you would need to usegrid
on thefigure
so that you could place each item properly like on the layout.- The
verified buyer
should be usingp
tag and notspan
. Always put element on a meaningful html tag.
Those only. Again, the site looks really nice already and great job again on this one.
Marked as helpful1@ahmedbesheerPosted almost 3 years ago@pikapikamart Hi, glad to see you as well, and thanks for your words :).
I always keep worrying about BEM classes' names and I don't know why, but I think it will come naturally with more practice.
Now let's move to the good part, your notes were really useful and challenging at the same time 😂.
I updated the solution with all the things you suggested, you can take a look and let me know if there is anything I can improve even further.
and I appreciate the continuous support and providing new info every time you comment, so keep up the good work 👍👊.
1@pikapikamartPosted almost 3 years ago@ahmedbesheer Hey, glad to be back on you with this one.
At the moment, the site is still showing the previous version that I gave a feedback to. Have you pushed the update on github?
Marked as helpful1@ahmedbesheerPosted almost 3 years ago@pikapikamart Yes, I have and the new version is live.
try viewing it from another browser or clear the cache from your current browser, because it happened to me before too and drove me mad 🤣.
0@pikapikamartPosted almost 3 years ago@ahmedbesheer Hey, I am now seeing the changes, that's weird about the first one though hahaha
As again, the site looks great as before and you placed each star icon neatly on the
background
great!One thing though, on the
figure,
like what I suggested on the markup previously, thefigcaption
should be the last item of thefigure
or it could be the first. So on this, swap theblockquote
andfigcaption
so that it will be validfigure
.So that's it, the site is really great and thanks for implementing those suggestions!!
Marked as helpful1@ahmedbesheerPosted almost 3 years ago@pikapikamart Chrome has always done this to me, but now I used to clear my cache if I noticed something off, so welcome to the club 😂.
and thanks for your checking and your suggestions, I'll make the
figcaption
the last item of thefigure
, I think I misread yourHTML
structure.have a good :)
1 - For each of the review-list-item, you could use the stars as value for the
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord