Design comparison
Solution retrospective
Finished this project and didn't liked it so much. I think I want to really learn and improve my newbie skill on JS.
So i was definitely not focused on this. I've probably forgot some things, but let me know in comments, I really enjoy your feedbacks !
Thx in advance !
Community feedback
- @pikapikamartPosted about 3 years ago
Hey, great work on this one. Desktop layout looks great, the site is responsive and the mobile state looks great as well. Great!
Some suggestions on the site would be:
- First, adding this stylings will help you on the layout:
html { box-sizing: border-box; font-size: 100%; } *, *::before, *::after { box-sizing: inherit }
This way, handling an element specially its size will be easier because of the
box-sizing
- It would be great to have a
max-width
maybe on thebody
tag so that the layout will have a limit. If you try to zoom out from your screen, you can see that the layout stretches and does not looks good, but having amax-width
will prevent the layout from stretching. - Use
footer
on the.attribution
so that it will have a landmark element. - For each of the
.social__hero__notes
selector, instead ofp
tag, usediv
for that since if you look at the component, it some contents thatp
tag alone can't handle. - Those star-images could have been used in the
background
property, let's say of the.social__hero__notes_{number}
. This way, you avoided usingimg
tags on the markup. Also just a quick reminder aboutimg
tag. When usingimg
tag, do not forget to add thealt
attribute, whether the value is empty or not. Because by not including it, screen-reader will instead read the source path of the image which you don't want. So always include it.
For the testimonial part:
- The person's image should be using the person's full name since it is already available. Use
alt="Colton Smith"
for example. - When using
img
tag, you don't need to add words that relates to "graphic" such as "image" and others, sinceimg
is already an image so no need to describe it as one. - The heading tag on the person's name is incorrect. When using a heading tag, make sure you are not skipping a level. If you use
h4
then make sure thath1, h2, h3
are all present. Use onlyh2
on the person's name. - The person's position should not be a heading tag since it doesn't really add any more information. Using
p
tag on the position would be great. - Also, if you would like. You can use this markup on each of the testimonial so that it will be easier for the user to navigate it properly when they are using like a screen-reader:
<figure> <img src="" alt={person name}> <blockquote> <p> {qoute in here}</p> </blockquote> <figcaption> person name <p> person role </p> </figcaption> </figure>
Though you would need to use
display: grid
on thefigure
tag so that you could place each item properly like in the design.Aside from those, great job again on this one.
0
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