3 columns card component using scss, bem methodology and flexbox
Design comparison
Solution retrospective
I would be glad to receive some feedbacks about my solution and any improvements that i could make.
Community feedback
- @correlucasPosted about 2 years ago
👾Hello Tiago, congratulations for your new solution!
I saw that you've used list to order the cards but in this case it's better to wrap the 3 cards with section and inside it, each card with
article
this is more semantic and in comparison with lists you need to write less code too and have more control over the content blocks.👋 I hope this helps you and happy coding!
Marked as helpful1@tiago-jv0Posted about 2 years ago@correlucas Thanks for the recommendation i will implement it right away!!
1 - @DavidMorgadePosted about 2 years ago
Hello Tiago, great job getting the solution for this challenge, congratulations!
Your project seems to be responsive at all widths but I would probably change the breakpoing for the desktop layout, because you are getting all in one column too early, probably using
@media(max-width: 768px)
instead of@media(max-width: 1440px)
would fix it!Also I think that leaving a bit of blank space on the mobile version instead of 100% width cards looks a bit more visually better to the user.
Hope my feedback helps you, if you have any questions don't hesitate to ask!
Marked as helpful1@tiago-jv0Posted about 2 years ago@DavidMorgade Thanks for the recommendations, i will change the media queries to use 768px, unfortunately, in the design the mobile view occupies 100% of device width, so, in order to follow 100% the design, i should not add padding to the sides
1 - @PhoenixDev22Posted about 2 years ago
Hi Tiago José,
Congratulation on completing this challenge. Your solution looks great. I have some suggestions regarding your solution if you don’t mind:
- Page should contain a level-one heading.To tackle the accessibility issue in the report: In this challenge , as it’s not a whole page, you can have
<h1>
visually hidden withsr-only
.
- In my opinion, the images are much likely to be decorative. For any decorative images, each img tag should have empty
alt=""
andaria-hidden="true"
attributes to make all web assistive technologies such as screen reader ignore those images.
- What would happen when the user click those learn more? In my opinion, clicking those "learn more" would likely trigger navigation not do an action so button elements would not be right. So you should use the
<a>
. For future use , it's a good habit of specifying the type of the button to avoid any unpredictable bugs.
- You should remove the
<ul>
.
Hopefully this feedback helps.
0 - Page should contain a level-one heading.To tackle the accessibility issue in the report: In this challenge , as it’s not a whole page, you can have
- @AdrianoEscarabotePosted about 2 years ago
Hello @tiago-jv0, how are you?
Congratulations on another project, it was really very good! I have some tips for you:
I noticed that each card has a 'ball' due to the fact that they are an unordered list, we can arrange it this way:
.cards { list-style: none; }
This will make the list without any decorations.
The rest is really good!
Hope it helps... 👍
0
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