Design comparison
Solution retrospective
I'm happy about the banner images and how I used background properties to deal with them. Not sure if it was the best approach or I should have added them using html.
I would like to write better css next time and have a proper arrangement for scss.
What challenges did you encounter, and how did you overcome them?I was struggling with finding out the best approach to add the banner images. I thought maybe I should add them using or maybe image-set but I wasn't sure and I wasn't able to achieve what I wanted so I just went with adding them using url() with background-image and then using media query to replace the image.
What specific areas of your project would you like help with?I would like help with this banner project thing what would have been the best apprach. Like should i have added them in html and then used media queries or maybe use or srcset or image-set?!
Community feedback
- @rupali317Posted 3 months ago
Hi @itsmesrishti
I have reviewed your code, and in my opinion, the way you implemented the banner image is very well done. Given that the image is slightly cropped at the sides, using url() with background-image and background-position is an appropriate approach.
I have the following suggestions for the CSS part:
- I noticed that in a couple of places you have defined font-size in the html tag. Avoid using pixel since it does not scale and is bad for accessibility. rem is preferable due to the ability to scale.
- In your variable file, avoid naming the variables like $purple-hover, $cyan-hover because what if the requirement decides to change the color theme from purple to red? That means the name "purple" is no longer valid and you will also have to change the name in various places. Choose a name which is more generic such as $primary-color, $secondary-color.
- Besides defining color as CSS variables, you can also define other properties such as typography, border-radius, shadows, spacing. You can refer to this CSS example
- Your project should have a CSS reset otherwise different browsers will apply their own default stylings. We want a consistent look and feel in all the browsers. Refer to this CSS reset article
Marked as helpful1 - @itsmesrishtiPosted 3 months ago
@rupali317 Thank you for the feedback!
You're right if I used em it would have scaled better and I wouldn't have to use this many media queries!
I do use a very basic three line reset but maybe I should look into it more!
Thank you for the thorough feedback! I'll apply the tips you gave in my next project! And thank you for sharing the resources as well!
1
Please log in to post a comment
Log in with GitHub
Join 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