@geomydas
Posted
Hi @Manshiporiya, I have recently just finished checking your code and it looks good but it seems to have few issues which can easily be fixed.
My Feedback
- You shouldn't worry about browser compatiblity that much. Check out this handy website for deciding if a property is outdated or not. You would only worry about browser compatibility on very niche features such as animating from
display: none
or stuff similar to that. - Consider using a CSS reset, it basicall makes your css look the same in most browsers and makes it more consistent aswell. You dont have to do anything that much since you only need to copy and paste it inside your code and you are all set. I reccomend using josh comeau or andy bell css reset as most people use them
- Learn to use the rem unit. The rem unit is typically used in place of px and it makes your websites more accesible as it scales with the users set font size in the browser settings. You would typically use px for small stuff such as borders, outlines and shadows and rem for the rest. To be more specific than that, you would use rem if you want to scale with the users set font size which is the case most of the time
- Never ever set font-sizes and media queries in px and instead use rem/em. Using px is an accesibility issue for this case and I won't dive deeper into that since you should check out this resource
- Your site needs a main landmark. All sites should have atleast 1 main landmark for enhanced accesibility, consider replacing the div with the class of card with a main tag instead
- Remove the media queries in the project. There is no need to do so. The font sizes dont even change from mobile to desktop, the paddings dont change either, no need to change the max-width as it will resize fluidly and automatically. Also, your styles in media queries do nothing since you are using the id selector instead of the class selector for the card
- Don't set font-family in the global selector. Just set it on the body and all of the elements will inherit it automatically, you would only reserve the global selector for setting box-sizing to border-box and resetting either margins or paddings.
- Don't use descendant selectors that often such as
.card p
as that will increase the specificity and therefore making it harder to override the styles in the future, even if you might think "oh, i wont override this" in most projects I have seen it being override or I completely forget it. - Take a look at the BEM naming methodology and see if it helps you write better class names inside the HTML and write more better and maintainable CSS. It's not for everyone thought as it is verbose but I promise you to try it and you will fall in love immediately
- Remove the comments in the top part of the HTML. I don't see the need for it as it does not convey any meaning whatsoever. Comments are typically used as tips or warnings for future people who will take a look at your code. The comment at the top of your HTML however is just a copy of the document without the HTML tags.
It might seem overwhelming at first but treat it as a checkbox and you'll get it done in no time. Have a nice day and have fun coding!