Design comparison
Solution retrospective
This was my first responsive page design task and I feel like it went pretty well, for the majority of tutorials and reading I've done they said that using fixed widths for containers was a bad idea so I feel like I may have committed a cardinal sin at some point?
My main focus was to get flexbox working as intended, however I feel like an additional media query at a tablet-sized screen may have been useful as the design looks poor between 700-900px
As I said this is my first project so feedback is most certainly welcome and I appreciate any time you take looking through my solution.
Cheers, Dan
Community feedback
- @HassiaiPosted almost 2 years ago
Replace<div class="container">with the main tag and <div class="attribution"> with the footer tag to fix the accessibility issues. click here for more on web-accessibility and semantic html
To center .container on the page, add min-height:100vh; display: flex; align-items: center: justify-content: center; or min-height:100vh; display: grid place-items: center to the body.
To center .container on the page using flexbox: body{ min-height: 100vh; display: flex; align-items: center; justify-content: center; }
To center .container on the page using grid: body{ min-height: 100vh; display: grid; place-items: center; }
Hope am helpful.
Well done for completing this challenge. HAPPY CODING
Marked as helpful1 - @MelvinAguilarPosted almost 2 years ago
Hello there π. Congratulations on completing your first challenge! You have done a great job and I can see you are on the right track.
I have some suggestions about your code that might interest you.
HTML π:
- Use the
<main>
tag to wrap all the main content of the page instead of the<div class="container">
tag. With this semantic element you can improve the accessibility of your page.
- Use the
<footer>
tag to wrap the footer of the page instead of the<div class="attribution">
. The<footer>
element contains information about the author of the page, the copyright, and other legal information.
- You must use a level-one heading (h1) even though this is not a full-page challenge. You can create an '<h1>' element within your 'main' element that will be hidden visually but visible and readable by screen readers. The class "sr-only" hides content visually and here are the styles to copy. e.g.:
<h1 class="sr-only">3-column preview card component</h1>
- You should use the
<a>
tag instead of the<button>
tag because theLearn More
button is a link to another page. Use buttons to perform actions like submitting a form or closing a modal and use links to navigate to another page. You can read more about this here π.
-
Not all images should have alt text. Car icons are for decoration purposes only, so they can be hidden from screen-readers by adding
aria-hidden="true"
and leaving its alt attribute empty:<img src="./images/icon-sedans.svg" alt aria-hidden="true"> <img src="./images/icon-suvs.svg" alt aria-hidden="true" > <img src="./images/icon-luxury.svg" alt aria-hidden="true" >
CSS π¨:
- Instead of using pixels in font-size, use relative units like
em
orrem
. The font-size in absolute units like pixels does not scale with the user's browser settings. This can cause accessibility issues for users who have set their browser to use a larger font size. You can read more about this here π.
- To center the component in the page, you should use Flexbox or Grid layout. You can read more about centering in CSS here π.
body { min-height: 100vh; display: flex; justify-content: center; align-items: center; flex-direction: column; }
I hope you find it useful! π Above all, the solution you submitted is great!
Happy coding!
Marked as helpful1 - Use the
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