Design comparison
Solution retrospective
The biggest reward is your feedback
Community feedback
- @pikapikamartPosted about 3 years ago
Hey, great work on this one. Layout in desktop seems good, but the box is just a bit on the left side and the whole layout is not centered. On mobile state, the top part of the image gets hidden by the screen.
Just some additions to what dewslyse already said:
HTML structuring:
- When you create a website, always have a
main
element that wraps the main content of that page like on this one. Havingmain
element helps other users to navigate properly on your website. - The images could have used an empty
alt
text. When you think an image is just for decorative purposes only, you should make thealt
to bealt=""
. This makes assistive tech to skip that image, because it doesn't really means anything. - On the
FAQ
section, usingdiv
alone as the toggler or container for those question is not really accessible or good at all. Since this is a dropdown, you can use thedetails
element which is a native element that is really good for this kind of sections. It already have native accessibility that is why I recommend it. If you want another, you can make usebutton
and have aaria-expanded
attribute being set to it. You can search for the latter one. Butdetails
is enough. - The
alt
of the arrow icons on the dropdown should be left empty,alt=""
.
For css
- Don't apply any stylings in the
html
element. Don't. :>. Remove all of those stylings from the html aside from thefont-size
andbox-sizing
leave it there. The rest remove them. - I don't really recommend applying a fixed
height
andwidth
on thebody
tag. On your case, better leave it at default, then just apply those fixed dimensions on the.project-container
. To make it better, on yourbody
tag, have these stylings:
body { margin: 0; min-height: 100vh; # do not use height: 100vw display: grid; place-content: center; # centers the content; /* add the linear-gradient background that you set in the html element */ padding: # you can set how much padding to use, especially in top and bottom remove these stylings from the body: /* position: fixed; */ /* top: 25%; */ /* left: 25%; */ /* width: 900px; */ /* height: 500px; */ } .project-container { # this is where you should put the fixed dimensions. Also add a border-radius /* add the background image of the pattern in here */ height: 500px; width: 900px; position: relative; background-color: white; border-radius: 8px; }
But remember, this is for the desktop layout. On mobile it will change, but at least you have a more clearer stylings. Sorry about that hehe:> , but hey, you can just tinker a bit on the mobile media query.
Still, you did a good job in here. Also, you don't have to follow those css stylings that I mentioned, I just said it so that you will have an idea on where css should have. Like the
html
should not have any stylings, thebody
should now have those custom fixed dimensions. Refactoring css will be really good.Still, good work on this one.
1 - When you create a website, always have a
- @dewslysePosted about 3 years ago
Hello @admase! Congrats on your submission. Looks good. The accordion items overflow the card when all items are opened. Aside that everything looks fine. You may also want to resolve the accessibility issues raised in the report. Happy coding
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