My second submission on Frontendmentor. Any feedbacks are greatly appreciated! ππ»
Noemi Gali
@NomiDomiAll comments
- @cornthecobSubmitted almost 3 years ago@NomiDomiPosted almost 3 years ago
Hey @cornthecob,
Congrats on completing your second challenge! :)
Here are some tips on my side:
- avoid putting heights on your tags, classes etc. Let the height be adjusted automatically by the content inside
- for a smooth transition from desktop to mobile avoid using pixels for most things. Here is a great tutorial that talks about the different types of units that you can use
- use CSS variables for your font family, sizes, margins and paddings too, not just for your colors
- give more generic names to your color custom properties. Instead of
--VeryPaleBlue
use--primary-clr
. In case you decide to change your colors, your --VeryPaleBlue might have to be green or red and then your variable name will be very confusing - for easy access to your code consider putting your html and css files in a folder named src. Src stands for source and it is usually used to easily distinguish between the code and additional helper files. Also add all the images you use in your code in a folder named images or assets inside your src folder
Hope this helps! Keep on coding! :)
0 - @4gotpwdSubmitted almost 3 years ago
Any feedback will be highly appreciated. thank you!
@NomiDomiPosted almost 3 years agoHey @4gotpwd,
Really great job with picking your class names. Your html is general and easy to understand.
For the css part I would encourage you to add not just the colors to your :root selector, but the font family, sizes, paddings, margins etc. Here is a great tutorial that explains why and how.
I see you used different types of units which is great! If you want a better and smoother responsiveness, try focusing on using other units than pixels for margins, paddings and font sizes. You can see here what the best practices are.
Overall great job! :) Hope this helps! Keep coding!!
Marked as helpful2 - @Bechara-AadSubmitted almost 3 years ago
Hi everyone!
1st challenge! putting my HTML/CSS learning to the test.
Through this challenge, I appreciated learning more about manipulating the background in the body for it to dynamically adapt to different screens.
One small niggle that I noticed, I believe the white border around the profile image in the card is not completely 100% enclosing the image. As if there is a very thin circle in-between the white border and the image where I can see through into the backround. This effect is more visible on some zoom levels in the browser.
How can this be corrected in code? Also, thank you for reviewing my solution and offering any instructive feedback.
The support of this community is truly appreciated, and I'm looking forward to more challenges that help us grow together.
Bechara,
@NomiDomiPosted almost 3 years agoHey @Bechara-Aad,
First I'd like to congratulate you on completing your first challenge! :) Great job!
You are completely right about the border not enclosing 100% the image. One way around this is to put a
background-color: white
in the same class where you set the border. In your case.profile { border: 0.3rem solid white; border-radius: 50%; position: relative; bottom: 1rem; }
becomes
.profile { border: 0.3rem solid white; border-radius: 50%; position: relative; bottom: 1rem; background-color: white; }
This works great for this project, but sometimes it might not be sufficient. Read this for other methods of dealing with the issue.
Now looking at your code, here are some additional tips on my side:
- best practice is to use kebab case when naming your classes. So basically your class
card__header
should have been written likecard-header
- great job on not using divs everywhere. It's really great to see you picking things like
<header class="card__header">
instead. However if you use html tags like this, there is no need to give them a class name same as the html tag. Just simply write<header>
and put your css style on header instead of the class .card__header - even though I am happy to see you trying out other tags than div, the use of header and footer in this case could have been replaced with section. Headers and footers are usually part of a page (like in Word). They aren't really used for inside a card container like in this case
- I see that you are having your styles divided between your html and css files. Best practice is to keep everything in one place (in the css file), so in your case move the style section from your html in your styles.css file.
- for clean code, I would suggest you take a look at CSS variables. On big projects this is a must. :)
- great use of the different type of units. Keep that up!
Hope this helps! Again, great job and keep coding! :)
0 - best practice is to use kebab case when naming your classes. So basically your class
- @lucasweidasSubmitted almost 3 years ago
Any feedback will help a lot!
@NomiDomiPosted almost 3 years agoHey @lucasweidas,
You did an amazing job on this one! :) Good use of custom variables and the different type of units.
I would encourage you to add also the font sizes and paddings in the :root selector for even cleaner code.
Html wise, best practice is to use kebab case on all class names. I saw that some of your classes are named
social__number
whereas best would be to be written likesocial-number
.I would also argue that you can rewrite this:
<div class="social-info"> <p class="social__number">80K</p> <p class="social__title">Followers</p> </div>
like this
<div class="social-info"> <p class="number">80K</p> <p class="title">Followers</p> </div>
It keeps your class names smaller and in big projects this can be very handy. :)
Hope this helps! Keep on coding! :)
Marked as helpful1 - @ValeriaMontoyaSubmitted almost 3 years ago
Any feedback is well received π
- @diegop985Submitted almost 3 years ago
Hi Everyone, I've been practicing HTML-CSS nonstop and I think the results are finally starting to be noticeable, for this challenge I took the CSS code order/aesthetic very serious. I start using BEM and try really hard on responsive. If you want, pelase take a look and leave a feedback, it really helps me. π
@NomiDomiPosted almost 3 years agoHey @diegop985,
Really good job! It is very obvious that you've been looking into things.
Good use of the different types of units and CSS selectors.
Here are some tips on my side:
- try to keep your class names even more generic. I can see that for the different type of cards your chosen names are
card--orange
,card--blue
,card--green
. Try usingcard-1
,card-2
,card-3
instead. The reason for this is because in a production environment maybe you change your mind and you choose completely different colors for your card. The orange card becomes pink, the blue card becomes purple and the green card becomes blue. You can see the confusion that might create. Now think of if you work in a team and someone else comes and looks at your code. They might go straight to your CSS and change things for your card--blue, but instead of changing the current blue card they will be changing the purple one. - I see you decided to used different type of name conventions for your class name: card--orange, card__icon. Best practice wise you should use the kebab case where you put only one dash - between your words. So if I were to rewrite your classes card--orange becomes card-orange and card__icon becomes card-icon.
- it is amazing that you are using CSS variables and you take your colors from there. I would encourage you to do the same for your font family, sizes, weights etc.
Here are some useful videos:
Hope this helps! Keep coding! :)
Marked as helpful3 - try to keep your class names even more generic. I can see that for the different type of cards your chosen names are
- @OsumiiSubmitted almost 3 years ago
Beginner here: Where could I improve to make the code cleaner and more efficient?
@NomiDomiPosted almost 3 years agoHey @Osumii,
One best practice is to not specify the height of anything if possible. The height should be auto, therefore not even mentioned. It should automatically increase based on the items within.
I would not use ul and li tags the way you did. All of those could be divs. The use of ul and li is different. Check this for a better understanding.
For example for the nft-card-creator you could have written things like:
<div class="nft-card-creator"> <img src="./images/image-avatar.png" alt="Creator"> <div class="name">Creation of <span>Jules Wyvern</span></div> </div>
Great job on using CSS variables. I would encourage you to put font sizes and weights there too.
I see you using vw for font sizes in
font-size: 1.2vw;
. Use rem, em, px, never vw or vh.Here are some very good videos that will give you a solid base you could continue building on:
- Are you using the right CSS units?
- CSS em and rem explained
- Improve your CSS by Keepin' it DRY
- CSS variables
Hope this helps!! Keep coding! :)
Marked as helpful0 - @juangin3rSubmitted almost 3 years ago
Hello, recently I started to learn programming and I would like that anything that can be done better, tell me so I improve my code, thank you
@NomiDomiPosted almost 3 years agoHey @juangin3r,
Welcome to the world of coding! Congrats on completing the challenge!
Looking at your solution your images do not show up. That is because you are trying to bring them in like
<img src="/images/image-equilibrium.jpg"
. You need to specify your starting point by putting a point (.), therefore a first fix would be to change the aforementioned to<img src="./images/image-equilibrium.jpg"
. The point symbolizes here, which is your current folder.One best practice is to not specify the height of anything if possible. The height should be auto, therefore not even mentioned. It should automatically increase based on items within.
I can see that when it comes to the width you set the initial width to 100vw which is great, but try doing that for your other components. Use percentage or other units than pixels. This will make your design responsive and won't lock stuff in place as pixels tend to do.
For cleaner code, I would suggest to take a look at CSS variables. Here is a great video on what they are and how to use them.
Other useful videos:
Hope this helps! Keep coding! :) You are doing great!
Marked as helpful0 - @ganeshcool1Submitted almost 3 years ago@NomiDomiPosted almost 3 years ago
Hey @ganeshcool1,
Congrats on completing your first FrontendMentor challenge! :)
Here is some feedback on my side:
- try using other html tags than div. You have the following
<div class="footer"></div>
. If I were you, I would just use the<footer></footer>
tag. <p class="header">Equilibrium #3429</p>
is not really a paragraph. It is a good idea to distinguish paragraphs from titles. Titles are usually put within headers. In this case h1 would be a good fit.- think about using CSS custom variables to store colors and font sizes. This will keep your code clean.
- I can also see that you didn't use the suggested font. In case that wasn't on purpose, there are different ways you can import a font. One way is to put
@import url('https://fonts.googleapis.com/css2?family=Outfit:wght@300;400;600&display=swap');
at the top of your css file and then make sure to have this font used by your entire project by adding* { font-family: font-family: 'Outfit', sans-serif; }
, where * stands for all and it will apply this font to all your tags.
Hope this helps! Keep coding! :)
Marked as helpful0 - try using other html tags than div. You have the following
- @syzwnftrSubmitted almost 3 years ago
Any feedback for improvement are greatly appreciated.
- @anoshaahmedSubmitted almost 3 years ago
I would appreciate opinions on the organization of the HTML and CSS code. Also, if there is a more efficient way to do something than what I chose to do.
@NomiDomiPosted almost 3 years agoHey @anoshaahmed,
Great job on your first design!
I checked your other solution too and I saw that you improved tremendously from one design to another so really good job on that!
I would encourage you to use not just colors in your :root selector but also font size, font family, font weights etc. The idea of having CSS custom properties or variables is to keep everything in one place for cleaner code and easy changes. This might not make so much sense as of now on such small projects, but when you work in big projects this is a must.
I would also encourage you to take a step away from using pixels for margins, font sizes etc. Pixels don't allow for smooth responsiveness and will lock stuff in place.
Here are some great tutorials on the different types of units:
FYI I took inspiration from your design for the image overlay so good job on that!
Hope this helps! Great start and just keep on coding! :)
Marked as helpful2 - @Naveed89-techSubmitted almost 3 years ago
Any Feedback will be appreciated.
@NomiDomiPosted almost 3 years agoHey @Naveed89-tech,
Really great job! You seem to have a really good understanding of the different types of units one can use in CSS.
The only suggestion I could give is to create CSS variables in the :root selector and use them for cleaner code.
Check this out in case you haven't use them before.
Hope this helps! Keep coding! :)
Marked as helpful1