Design comparison
Community feedback
- @AdrianoEscarabotePosted about 2 years ago
Hi Kevin Mainar Baguioso, how are you?
I really liked the result of your project, but I have some tips that I think you will like:
1- Document should have one main landmark, you could have put all the content inside the
main
tag click here2- All page content should be contained by landmarks, you can understand better by clicking here: click here
I noticed that there were two scrollbars on the page, to fix this we can do the following.
I added:
* { margin: 0; padding: 0; }
The rest is great!!
Hope it helps...👍
Marked as helpful1@tomfge02Posted about 2 years ago@AdrianoEscarabote Thanks that's very good to know. I was wondering can you put div as a child of the parent element?
1@AdrianoEscarabotePosted about 2 years ago@tomfge02 If the parent element is the body, it is recommended not to!
We have to make sure that all content is contained in a reference region, designated with HTML5 reference elements or ARIA reference regions.
Example:
native HTML5 reference elements:
<body> <header>This is the header</header> <nav>This is the nav</nav> <main>This is the main</main> <footer>This is the footer</footer> </body>
ARIA best practices call for using native HTML5 reference elements instead of ARIA functions whenever possible, but the markup in the following example works:
<body> <div role="banner">This is the header</div> <div role="navigation">This is the nav</div> <div role="main">This is the main</div> <div role="contentinfo">This is the footer</div> </body>
It is a best practice to contain all content, except skip links, in distinct regions such as header, navigation, main, and footer.
Link to read more about: click here
0 - @hyrongennikePosted about 2 years ago
HI,
Congrats on completing the challenge
If you want center the columns or any child element you can use flexbox. You can add the following rules to your CSS file or make the appropriate change to yours.
body { display: flex; flex-direction: column; justify-content: center; align-items: center; min-height: 100vh; } .main-container { margin: 0 auto; }
Hope this helpful. Let me know if you have any other questions.
Marked as helpful1@tomfge02Posted about 2 years ago@hyrongennike Thank you for the feedback. I'll keep this in mind for the future practices.
0 - @correlucasPosted about 2 years ago
👾Hello Kevin, Congratulations on completing this challenge!
I saw your solution preview site and I think it's already really good. Here’s some tips for you to improve it:
You made your html structure entirely with
div blocks
but these div doesn't any semantic meaning, for this reason is better you use a better html markup improving your code, for example for each vehicle card you use<article>
instead of the<div>
.To make your CSS code easier to work you can create a
single class
to manage the content that is mostly the same for the 3 cards (paddings, colors, margins and etc) and another class to manage the characteristics that are different (colors and icon), this way you'll have more control over then and if you need to change something you modify only one class.✌️ I hope this helps you and 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