Design comparison
Community feedback
- @hernanruscicaPosted about 2 years ago
Hi @SrHatcher . It's great that you wants to improve yours skills, and I think this is the best place. Comments from the code:
- You could use BEM style for the classes, since you almost did it, but with one -. BEM is with two underscores ___ separation.
- I like the "object-fit: contain;".
- must to use the font size that the disegn give you.
- the 375 PX width for mobile that isn't means that the container has to be that size. It give you an idea of the size of the qr-container . But after all, great job, keep going! Greetings!
Marked as helpful1@SrHatcherPosted about 2 years agohola @hernanruscica!
I've heard from BEM a while ago but i never researched about it. i think it's time to get to know it! I'll apply the correction to the font size and i used the 375px because i knew that that'll be the perfect size for all platforms. Muchas gracias por el feedback Cesar! n.n
1 - @VCaramesPosted about 2 years ago
Hey @SrHatcher, great job on this project!
To help keep your CSS code organized and easier to use, I suggest implementing CSS Variables. This will come in handy when building large websites, using light/dark mode, etc…
It’ll look something like this:
:root { --primary-color: value; --secondary-color: value; --tertiary-color: value; }
And to use the variables you’ll use the var() function. So it’ll look like this.
h1 { color: var(—primary-color); }
Heres are some articles regarding CSS Variables.
You can also take a look at my projects and see how I use it.
https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties
https://www.w3schools.com/css/css3_variables.asp
Happy Coding!
Marked as helpful1@SrHatcherPosted about 2 years agohallo @vcarames!
i've known the existence of the CSS variables from some months agos but i haven't used them much, This information will help me get to used it. I appreciate your comment a lot.
Thank you so much for the feedback! n.n
0 - @correlucasPosted about 2 years ago
👾Hi @SrHatcher, congratulations on your first solution!👋 Welcome to the Frontend Mentor Coding Community!
Great solution and a great start! From what I saw you’re on the right track. I’ve few suggestions for you that you can consider adding to your code:
1.Use
<main>
instead of<div>
to wrap the card container. This way you show that this is the main block of content and also replace the div with a semantic tag.2.Use relative units as
rem
orem
instead ofpx
to improve your performance by resizing fonts between different screens and devices. These units are better to make your website more accessible. REM does not just apply to font size, but to all sizes as well.3.Save your time using a CSS RESET to remove all default settings that are annoying as the margins, paddings, and decorations and optimize it making it easier to work, see the article below where you can copy and paste this CSS code cheatsheet: https://piccalil.li/blog/a-modern-css-reset/
✌️ I hope this helps you and happy coding!
Marked as helpful1@SrHatcherPosted about 2 years agohello @correlucas ! I didn't know that such a tool like CSS RESET existed. i'll take a look and i'll apply the suggestions that you comment. thank you so much for the feedback.
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