Submitted almost 2 years ago

3 Column Preview Card with Basic HTML and CSS

Felix•60

@Farrukh997

Preview site View code

This is a solution for...3-column preview card component

HTML
CSS

1newbie

Design comparison

SolutionDesign

Solution retrospective

I would be happy if you give some feedback upon I cam improve my skill.😉😎

Community feedback

バレンタイン 😈•64,170
@VCaramesPosted almost 2 years ago
Hey there! 👋 Here are some suggestions to help improve your code:

The “car images/icons” in this component are purely decorative; They add no value. So their Alt Tag should be left blank and have an aria-hidden=“true” to hide them from assistive technology.

More Info:📚

https://www.w3.org/WAI/tutorials/images/

The headings in your component are being used incorrectly. Since the h1 heading can only be used once, it is always given to the heading with the highest level of importance. This component has three headings of equal importance, so the best option would be to use an h2 heading since it is reusable and it will give each heading the same level of importance.

If you have any questions or need further clarification, feel free to reach out to me.

Happy Coding!🎄🎁
Marked as helpful
1
Felix•60
@Farrukh997Posted almost 2 years ago
@vcarames, thank you for your feedback. I didn't know images could have a context and the rule about h1. I'm glad you corrected me.
0
バレンタイン 😈•64,170
@VCaramesPosted almost 2 years ago
@Farrukh997

Glad I could help!

Keep it up!
0
Adriano•38,100
@AdrianoEscarabotePosted almost 2 years ago
Hello Felix, how are you? I truly loved your project's outcome, however I have some advice that I hope you'll find useful:

I noticed that you used a button in which case the best option would be an a, because in my head when a person clicks on a button written Learn More, he is not confirming a form, or something like, it will be redirected to another page, to Learn More about!

to solve this problem do this:

<a href="/">Learn More</a>

You have used <br> , using <br> is not only bad practice, it is problematic for people who navigate with the aid of screen reading technology. Screen readers may announce the presence of the element. This can be a confusing and frustrating experience for the person using the screen reader. You can read more in MDN.

The remainder is excellent.

I hope it's useful. 👍
Marked as helpful
1
Felix•60
@Farrukh997Posted almost 2 years ago
@AdrianoEscarabote, thanks for your feedback. I didn't suspect that Learn More could be a link.
1
👾 Ekaterine Mitagvaria 👾•7,600
@catherineisonlinePosted almost 2 years ago
Looks great! Make sure to add cursor: button for buttons and maybe some transitions for smoother color change
Marked as helpful
1
Felix•60
@Farrukh997Posted almost 2 years ago
@catherineisonline, thank you for your feedback. I've implemented a transition for the color looks better.
0
Sinisa Vukmirovic•1,690
@SinisaVukmirovicPosted almost 2 years ago
Hello!

I see you are using some semantic elements but not all. Your reoort errors are mostly about that.

This "Document should have one main landmark" means you are missing <main> semantic element in your html. Landmark just means element.

This "All page content should be contained by landmarks" means all non-semantic elements should be inside of semantic elements, and not directly inside of <body> element.

Try to use semantic elements in your HTML. More about semantic elements

Hope this helps your in fixing code errors.

Best of luck!
Marked as helpful
1

Please log in to post a comment

Join 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