Design comparison
Solution retrospective
In my view it was a simple project to start my career, I had no difficulties, the project is visibly close to the challenge, I can't say if I did the development correctly and not to say the patterns in question of typing the codes.
If you can evaluate and mention points that I can improve, thank you!
Community feedback
- @imadvvPosted over 1 year ago
Greeting @HeroLeam!! Congratulations for completing your challenge!, 👏👏👏. well done
you did great, and you can also add
cursor: pointer;
to the buttons, as you may notice that buttons doesn't have a cursor pointer by default,button { cursor:pointer; }
overall good attempt, keep it up and have a good day/night
Marked as helpful0 - @0xabdulkhaliqPosted over 1 year ago
Hello there 👋. Congratulations on successfully completing the challenge! 🎉
- I have other recommendations regarding your code that I believe will be of great interest to you.
HEADINGS ⚠️:
- This solution consists incorrect usage of
<h1>
so it can cause severe accessibility errors due to incorrect usage of level-one headings<h1>
- Every site must want only one
h1
element identifying and describing the main content of the page.
- An
h1
heading provides an important navigation point for users of assistive technologies, allowing them to easily find the main content of the page.
- In this solution there's three
<h1>
elements, like this<h1>SEDANS</h1>
, you can preferably use<h2>
instead of<h1>
. Remember<h1>
provides an important navigation point for users of assistive technologies so we want to use it wisely
- So we want to add a level-one heading to improve accessibility by reading aloud the heading by screen readers, you can achieve this by adding a
sr-only
class to hide it from visual users (it will be useful for visually impaired users)
- Example:
<h1 class="sr-only">3-column preview card component</h1>
- If you have any questions or need further clarification, you can always check out
my submission
for this challenge where i used this technique and feel free to reach out to me.
.
I hope you find this helpful 😄 Above all, the solution you submitted is great !
Happy coding!
0 - @vanzasetiaPosted over 1 year ago
Hi, HeroLeam! 👋
Some suggestions for improvements.
- Remove extra elements: Style the heading element instead of the
<div class="title-sedans">
. Also, remove the<div>
elements that wrap the "Learn More" buttons. - Decorative images should not have alternative text: The car icons should have empty alternative text. This will tell the screen reader to skip over the decorative images. As a result, it saves screen reader users time navigating the page. For your information, decorative images are images that do not add any information and serve only aesthetic purposes.
- Use CSS to uppercase the text: Screen readers might spell the uppercased word in the HTML (spelled letter by letter).
- Wrap the text with a meaningful element: For example, use a paragraph element to wrap the text. There should not be text in
<span>
and<div>
alone. - Use one stylesheet: For this size of project, I recommend having one CSS file. This can reduce the requests and improve the site's performance. Also, it makes easier to review all your CSS code.
I hope my feedback helps. Have a nice day! 😄
0 - Remove extra elements: Style the heading element instead of 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