@mkboris
Posted
Great work @msakuma-dev, here are a few things to review
Here's how you could improve the BEM naming The .content class could be more descriptive, you could name it .qr-card, qr-card__image instead of just img, qr-card__title instead of just h1, qr-card__description instead of just p
- To properly center the card, add
min-height: 100vh;
andjustify-content: center;
on thebody
. - Avoid setting fixed
widths
andheight
on elements, as this can create problems with responsiveness and content fit. Instead, let the content and padding determine the element’s size. If necessary, usemax-width
ormin-height
, and prefer relative units like rem for better adaptability. Update the .content width to usemax-width
and should be defined inrem
. Also, remove theheight
completely - Use a separate file for your CSS as it improves maintainability.
- Font-size should be written in
rem
not px. This article explains it better Why font-size must NEVER be in pixels. - Consider using a modern CSS reset at the start of the styles in every project. Like this one Modern CSS Reset. This will help reset a list of default browser styles.
Hope this helps, Good luck
Marked as helpful
@msakuma-dev
Posted
@mkboris , thank you so much for your insightful review! I'll try to improve the code with your suggestions after reading the articles you shared.
@msakuma-dev
Posted
@mkboris , how do you treat the border-radius units? rem as well?