Design comparison
Solution retrospective
Hello Community,
I tried to position the background images to the best of my knowledge, but would be grateful if I am told the best way to go about it, as it actually took me a whole day to get this challenge done.
Thank you.
Community feedback
- @didyouseekyngPosted over 3 years ago
Hey, Debbie... Good morning I actually like how you made your background image fixed, honestly... I struggled with the background positioning, I'm hoping you can help me look at what I did and actually help with the background positioning by creating a pull request on Github. So back to your code, I've gone through it, and this is what I have to say;
-
I noticed you didn't use the semi-colon here which is something you have to watch out for when there's an error.
background-size: 700px, 700px; background-color: hsl(185, 75%, 39%)
-
It's something I learnt recently but I wouldn't mind if you learn it too, and it's based on naming of classes, I had to learn how to use the BEM naming style in order for me to name classes better, the class names are long but it actually makes it easier for you to work with and for people to understand your code easily. You can try it out and see if it works for you.
-
Regarding font size units, which is also something I'm guilty of but I'm changing now, You'd have to use rem for font size, like what I do now is that I downloaded an extension in my code editor which converts px to rem. So you could actually do the same.
<div class="figures"> <div> 80K </div> <div> 803K </div> <div> 1.4K </div> </div> <div class="info"> <div id="followers"> Followers </div> <div id="likes"> Likes </div> <div id="photos"> Photos </div> </div>
-
Maybe you didn't need the div with the info class, it could have been inside the div with figures class and then you use display flex on them. I know we can learn from each other, so feel free to look at how I code, tell me what you think, I'll be expecting a positive response.
-
Before I forget, try to always use the alt attribute with your img tag for accessibility. I've gotten used to this now, you will also have to do the same.
-
Try to actually limit your usage of the ID selector as you know it is very unique. Try using classes instead.
0@Deborah-OdionPosted over 3 years agoHello didyouseekyng,
Thank you very much for your feedback, I will be correcting the error with the missing semi-column, and will also convert the pixels to rem.
Thank you for introducing the BEM naming style, will research on that.
Yes I could have taken out the info class, but when I styled using the flexbox, I wasn't getting the exact positioning of the bottom information (followers, likes, photos) as the demo, I then decided to create a different class and use the margin properties, so it could be styled to almost the same way the demo looks. But I know that doesn't make it a clean code.
Thank you for noting the alt attribute, I can see that I skipped that.
Will be pulling a request on Github soon.
Thank you.
0@didyouseekyngPosted over 3 years ago@Deborah-Odion you're welcome, let's keep pushing, I'm impatiently waiting for that request 😄😄
0 -
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