@ahmedhanyh
Posted
Hey @JoseEliasMorales! Great work!
-
Add a css reset to remove some of the default syles applied by the browser (user-agent stylesheet). You can use a small reset like this (by adding it to the very top of the stylesheet):
* { margin: 0; padding: 0; border: 0; box-sizing: border-box; }
to remove all margins, paddings, borders, and to include the border size when setting elements' dimensions. Or you can use a larger one like this one. There are more resets out there, you can look them up.
-
Use
gap
instead of margins/paddings to space the contents inside of each.card
element. -
Change the
.card__btn
from a<button>
to a<a href="#">
because a link is more suitable for this content. When we see 'Learn More` we expect to be redirected to another page, which is how a link behaves. -
Add
cursor: pointer;
to.card__btn
to change the cursor symbol when hovering over each card button.
Hope the review was helpful! Wish you the best with your future challenges!
Marked as helpful
@ahmedhanyh Thank you very much for the tips! I fixed the padding using gap, for this I gave the __card
element display:flex
. As for the button, I investigated what you told me and put it like this: <a href="#"><button></button></a>
.
What do you think?
@ahmedhanyh
Posted
@JoseEliasMorales You're welcome! Glad the review was helpful!
I think the content (the 'Learn More' button) is more a link that looks like a button, and that it's more semantically correct to use an anchor element instead of an actual button. Take a look at this answer on stackoverflow.
I think it's better to style the anchor element to look like a button. We may have to make it function like a button using Javascript, like executing it when a user gives it focus and presses space/enter.
Keep up the great work! Ask me any questions you have anytime.
Marked as helpful
@ahmedhanyh Thank you. Now I understand. Advice applied!