@pikapikamart
Posted
Hey, great work on this one. Layout is really great both mobile and desktop. Also the responsiveness is good.
Some suggestions would be that:
- on the
body
tag, you could remove theoverflow: hidden
declaration. I see that you use this to hide the extra spacing at the bottom right. To fix that, just add aalign-items: flex-start
on yourmain
tag. The reason that happens is that, at default,display: flex
gives the layout a defaultalign-items: stretch
This makes the layout or the item of the flexbox have the same size based on the largest or highest element inside of it. Since the highest (based on height
) is the image, the text on the left side gets that size as well. And since you are using position: relative
and top
on the container on the left side, it pushes it down and creating extra space at the bottom. align-items: flex-start
fixes that by only giving every element in the flexbox the height, based on the content of the element.
- your slider togglers, the arrows, it is better to use
button
on it, wrapping theimg
of the arrowinside the
buttonso that it have extra accessibility, like when you use
tab` on your keyboard to navigate. - The name of the person who said the testimonial could be wrap inside a heading tag like
h2
- Also, when creating website, it is convention to have atleast 1
h1
tag. On your solution, maybe adding ah1
tag inside themain
tag, having the text liketestimonials
or any descriptive text that describes your website. But thish1
will only be for screen reader users, so thish1
will have a sr-only class, you can search only for sr-class. This is just css stylings that are intended for screen reader users.
Overall, you did great on this, especially on responsiveness.
Marked as helpful
@fazzaamiarso
Posted
@pikamart Hello!! Thank you so much for your comprehensive feedback and time. There is a lot of things that I was unaware of and missed here. Hope you have a good day!!