@vanzasetia
Posted
👋Hi There!
👍 Good job! Now, your files are clean from development code.
Some feedback:
- For more semantic HTML, I recommend to swap the
statsdivwithuland wrap each item withliinstead ofdiv. - The stats number should not be heading tags. When you want to use heading tags, think of it like a putting title on document file. I recommend to use paragraph elements instead.
- There are two
headerdivs, I recommend to only use one of them and changed the image usingbackground-imageon@mediaquery or try to usepicturetag instead. - I recommend to keep the specificity as low as possible by selecting the element directly (without keep adding the parent class name). In this case, in my opinion it's possible to style any elements without any complex selectors.
/* ❌ Adding parent element */
.card .content__title {
/* some styles */
}
/* ✔️ Directly select the element */
.content__title {
/* some styles */
}
- Use
remor sometimesemunit, instead ofpx. Usingpxwon't allow the users to control the size of the elements based on their needs. - For letter spacing, it's recommended to use
emunit, so when the text get bigger the letter spacing will automatically get bigger too.
That's it! Hopefully this is helpful!
@vanzasetia Thank you again for this very detailled feedback. That's crazy, everything you say seems so logical once you reed it.
I'll, for sure, be more attentive to everything on next challenge ! Talk you later, hoppefully
@vanzasetia Ok, then ! If you want to continue to make feedback to my work, here's my solution for the next challenge !
I tried to follow as much advises you pointed. Just wanted to let you know that complex selectors seems to come from SCSS syntax. This kind of :
.card .content__title {
/* some styles */
}
When my code is not compiled, it generaly appears with only one class !
Thanks for interest, again !
@vanzasetia
Posted
@FlorianJourde That CSS or compiled code is coming from this Sass code:
.card {
text-align: center;
background: $dark-desaturated-blue;
max-width: 330px;
margin: auto;
border-radius: 10px;
display: flex;
flex-direction: column;
.content {
padding: 35px;
&__title {
margin-bottom: 15px;
}
The card element is the parent element and then you use the & as the parent selector, which is referring to .content. So that's why, it gets compiled to:
.card .content__title {
/* some styles */
}
What I recommend is that you don't need to nest the content inside the .card element.
.content {
padding: 35px;
&__title {
margin-bottom: 15px;
}
}
So, it get compiled to this.
.content__title {
/* some styles */
}
@vanzasetia Okay, I've got it. Nesting with SCSS is not always needed, I'll try to be careful, thanks !
