Looking to hire developers?
Hey everyone!
All feedbacks are welcome.
I would love to get some feedback about:
The HTML of my code. Is it fully semantics? Does that h3 tags that I setted inside de li items make sense?
I definetely still need to understand flexbox better. I would like a lot about feedbacks on that.
I struggled a litle to set de image the way I wanted. That flex-basis property wont let me sleep tonight.
Is my use of margin and padding correct? Any recommendation?
What about that cover image colour? Is that ok?
Thank you a lot! Wish you all a great day!
You did a good work respecting the semantic, I suppose one small thing to review for the card text section is the padding to try to make your work pixel perfect. Keep up the good work and motivation.
Have a great day
Marked as helpful
@newlomar
Posted
@hhamza1 hi Hamza!
Amazing! Thanks a lot for this feeback.
I wish you a great day too!
@afrussel
Posted
Good work. Using HTML5 tags in good way.
Marked as helpful
@newlomar
Posted
@afrussel Thanks a lot for that, Ahmed!
I really appreciate it.
Hello
I'm a bit short on time so made changes and added notes in devtools. Hopefully this still makes sense!
@media (min-width: 992px) {
.card {
/* align-items: center; */
/* margin: 0px 250px; */
max-width: 58rem;
}
main {
/* height: calc(100vh - 20px); */
min-height: calc(100vh - 20px);
// note: these styles should be on mobile too, not in a media query;
}
.main__section .card .card__text {
/* padding: 0px 140px 0px 50px; */
padding: 3rem;
}
.main__section .card .card__text p {
/* line-height: 1.4; */
}
}
.card {
/* align-items: center; */
/* margin: 30px; */
max-width: 30rem;
}
.main__section .card .card__image figure img {
object-fit: cover;
}
.main__section .card .card__text {
/* align-items: center; */
/* padding: 20px; */
padding: 2rem;
}
.main__section .card .card__text p {
line-height: 1.7;
margin-bottom: auto;
}
.main__section .card .card__text ul li h3 {
/* font-size: 10px; */
font-size: 0.9375rem;
// note: remove these h3s;
// note: 10px is way too small for text;
letter-spacing: 0.075em;
// note: is font family and weight correct on this?;
}
.main__section {
padding: 1.5rem;
}
figure {
// note: you should not be using figure unless the content needs a caption;
}
h2 {
// note: remove these h2s - use span or strong tag;
}
h3 {
// note: remove these;
}
Good luck!
Marked as helpful
@newlomar
Posted
@grace-snow Thanks a lot, Grace!
Your comments makes my day! :smile
@newlomar
Posted
@grace-snow I've updated the code with the figcaption information. I forgot to use it before.
It's ok to use the same text that is in the alt text of the image? And, there is a problem with setting its display to "none"?
@newlomar
Posted
@grace-snow Oh, another 2 questions:
Why dont use h2 and h3? I really tought that I placed this tags correctly.
@NewLomar because headings must be that - titles/headings for other content. Numbers will never make sense as a heading, they have to be read with their accompanying word to make sense.
Think about how you would write this content in a document (imagine with the styles turned off). That's what we have to think about when writing html. This would be a heading, a paragraph, 3 bullet points and an image, without doubt
@NewLomar and this should not be a figure element at all, just an image or picture element, no caption. The only purpose for using a figure element is when a visible caption is necessary.
For example, figure might contain a chart, an infographic, an image or even a blockquote - but only things that need to have a caption alongside them (visible)
@newlomar
Posted
@grace-snow I'm starting to understand and your explanations definitely are one of the reasons for that.
I made the correction, but I still don't fully understading. I will get it with time.
Thanks again.
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
