Design comparison
Solution retrospective
Any tips for how to round the corners on my "banner" image?
Also how do I make <h2>
and <div class="container">
a focusable element.
Thanks for the tips in advance :)
Community feedback
- @BlackpachamamePosted 11 months ago
You've done a good job!
Some details to comment:
- It's
"HTML & CSS foundations"
not"HTML & CSS foundations"
😅 - The problem with the banner borders is that you are using a
padding-top
and apparently this cancels out the top borders, instead usingmargin-top
:
.banner { border-radius: 10px; margin-top: 25px; width: 250px; height: 150px; }
Marked as helpful2 - It's
- @danielmrz-devPosted 11 months ago
Hello @D-Salkovic!
Your solution looks great!
I have a couple of suggestions for improvement:
- For semantic reasons, and since that is the main title of the screen, you can replace the
<h2>
with<h1>
.
The
<h1>
to<h6>
tags are used to define HTML headings.<h1>
defines the most important heading.<h6>
defines the least important heading. Only use one<h1>
per page - this should represent the main heading/subject for the whole page. Also, do not skip heading levels - start with<h1>
, then use<h2>
, and so on.- Also, instead of using
margin
to center the card, here's a better way that adapts better on all screen sizes:
You can apply this to the body (in order to work properly, you can't use position or margins):
body { min-height: 100vh; display: flex; justify-content: center; align-items: center; }
I hope it helps!
Other than that, you did a great job!
Marked as helpful1@D-SalkovicPosted 11 months agoThanks @danielmrz-dev, a great tip about centering my
<div>
:)0 - For semantic reasons, and since that is the main title of the screen, you can replace the
- @MelvinAguilarPosted 11 months ago
Hello there 👋. Good job on completing the challenge !
I have other suggestions about your code that might interest you.
- Regarding making elements focusable, you can enclose them in an interactive tag, for example, enclosing the h2 in a link. For the entire container, you can refer to this article that explains how to create a link that covers the entire card: Inclusive Components - Cards.
- Other suggestions include using the
<main>
tag to wrap the entire component, applying a CSS reset to eliminate default styles like the default margin on the body element, and attempting to center it using flexbox instead of using such a large margin.
I hope you find it useful! 😄 Above all, the solution you submitted is great!
Happy coding!
Marked as helpful1
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