@pikapikamart
Posted
Hey, great work on this one. The layout both desktop and mobile is good and it responds and scales very well on screen changes.
Also, great that you adopting certain methodologies, although I don't know what is SMACSS but hey, if you got it, that's really great.
Some suggestions would be:
- Always have a
h1
element on a webpage. On this one, theh1
would only be a screen reader text. It might seem new to you, but this is intended for screen reader users. Theh1
element would look like:
<h1 class="sr-only">Frontendmentor car collections examples</h1>
The sr-only
class is just a css stylings, you can search some up in google. This h1 would be the first text element inside the main
.
- Avoid using
height: 100%
orheight: 100vh
on an element. This will make the element's height limited to the viewport's height. On yourbody
andmain
remove those css styling. That is a bad practice. Instead, on yourmain
element. Add these:
align-items: center; # centers the content vertically
min-height: 100vh; # makes sure it have necessary height and will expand
- On the
section
elements, you don't need to usetransform
on those element. Let themain
flex container centers them properly, just addpadding
to a certain parent element. - An
a
tag doesn't have atype
attribute. Remove those,button
element andinput
have those.
Overall, it looks good. Great work again.
Marked as helpful
@ozangokdemir
Posted
@pikamart Your feedback is very clear and complete, so thank you for the time you spent on writing it. I will take all your suggestions in consideration and update my work according to your advices.
Thanks again for your kind help !