Hi @tenze21 👋
Great use of semantic HTML 👍 Very well-organized CSS file. Great job overall! It is a very well-done project! Congratulation! 🎉
I have a few minor suggestions:
- ⭐️ Heading size seems to be a bit small. Perhaps try
2.5rem. - ⭐️ Paragraph text is a bit off. Consider trying a line height of
1.645. - ⭐️ Buttons get misaligned on certain width (633px, 686px, etc). Add
align-self: flex-end;to your.linkclass. - ⭐️ On small mobile sizes (around 375px wide), the
max-width: 16remon the.wrapperclass causes an accessible amount of left and right padding. Consider replacing it withmin-width: 19.4375rem. These changes should make your transition more fluid and prevent shrinking to tiny widths. - ⭐️ Even thought use of
<h1>per<article>element is within HTML5 specification. It is believed that multiple<h1>elements per document might create some confusion for assistive tech users. I think the<h2>heading is more appropriate. To keep heading hierarchy, you could add visually hidden<h1>to your document, announcing to assistive tech users that they are looking at "Frontend Mentor project submission," for example:
<body>
<main class="wrapper">
<h1 class=".vusually-hidden">Frontend Mentor project submission</h1>
<article class="sedans modal">
<h2>Sedans</h2>
...
</main>
</body>
Visually hidden class:
.visually-hidden {
position: absolute;
width: 1px;
height: 1px;
padding: 0;
margin: -1px;
overflow: hidden;
clip: rect(0, 0, 0, 0);
white-space: nowrap;
border: 0;
}
Otherwise, very well done! 🚀 Impressive! 🎉 Keep it up! 👍 I hope you find my comments helpful 🫶
Marked as helpful
1
Tenzin Choeda• 210
@tenze21
Posted
Wow... Thanks a lot @solvman for your compliments and suggestions. They are really helpful I will implement it on my project. Your idea of adding a visually hidden <h1> sounds great it's my first time coming through this 😄. Thanks a lot 😊.
1
