QR Code Component Solution

Community feedback

• 

  P

  Kamran Kiani•2,740

  @kaamiikPosted 3 months ago

  Hi. Congratulation for doing this challenge. I have few notes which may improve your solution:

  1. Try to use a proper CSS reset at the start of your CSS style. Andy Bell and Josh Comeau both have a good one. You can simply search on the internet to find them.

  ---

  2. Your font-size and max-width should be in rem unit not px. You can read this article about it and why you should not use px as a font-size.

  ---

  3. For the .qr-code image no need to add width. Just add a max-width: 100% and let the browser decide to shrink the image.

  ---

  4. I'm not sure adding the line-height in percentage unit is true or not but most of the time we add the line-height unitless.

  ---

  5. In a real-world scenario, this card is part of the website, and you will not use the h1 heading here because the level-one heading is used only once on your page. Therefore, I suggest changing the h1 to h2 for better practice.

  ---

  6. Add extra padding to your body element so that the card does not stick to the sides when you shrink down your page.

  Marked as helpful

  0

  ---

  

  Park Byung Eun•40

  @ethan-geekPosted 3 months ago

  @kaamiik

  Hi kaamiik, Thank you for the detailed feedback. Your suggestions on using a proper CSS reset, rem units, and better heading structure were really helpful. I’ll also explore fluid typography and container queries for a more responsive layout. Thanks again for sharing your valuable insights!

  0
• 

  P

  Huy Phan•3,430

  @huyphan2210Posted 3 months ago

  Hi @ethan-geek,

  I saw your question about CSS and wanted to share my thoughts:

  You're doing a good job with CSS variables! I noticed your naming convention seems inspired by properties like font-weight, where a name like --slate-900 might imply a heavy shade of slate. While I personally haven’t named variables this way, it’s a valid approach if it works for you. However, if you're collaborating with others, it’s a good idea to discuss naming conventions to ensure everyone’s on the same page.

  Regarding the challenge: it’s a beginner-friendly task, and your styles.css looks good overall. Just a few suggestions for improvement:

  1. Use rem for better scalability:

  • Instead of px, consider using rem for font sizes and spacing. For instance, 1rem equals 16px by default, so 320px could be replaced with 20rem. For smaller values like 15px, you might keep them in px for precision, but aim to minimize px usage where possible.

  2. BEM (Block Element Modifier) naming:

  To enhance readability and scalability, you might consider adopting BEM. Here's an example using your current classes:

  • If you have .container at the top level, a card inside it could be named .container__card instead of just .card. Similarly, .text-container could become .container__card__text-container.

  That said, BEM can lead to long class names when elements are deeply nested. The goal of any methodology, including BEM, is to make your code more understandable and scalable. If it feels like overkill for your project, it’s perfectly fine to skip it. Always prioritize what works best for you and your team.

  3. Formatting:

  • Add consistent spacing between class selectors in your CSS for better readability.

  Hope this helps! Keep tackling challenges, and we might have another discussion.

  Marked as helpful

  0

  ---

  

  Park Byung Eun•40

  @ethan-geekPosted 3 months ago

  @huyphan2210

  Thank you so much for your detailed feedback!

  I really appreciate your advice on using rem to improve scalability and accessibility. It’s something I’ll definitely start incorporating into my projects. Also, your suggestions about BEM are very helpful—I’ll experiment with it and see how it works for me in terms of readability and scalability.

  Your thoughtful review inspires me to provide detailed feedback like you did. I’ll work on improving my skills to contribute more effectively in future discussions. Thanks again for taking the time to share your insights!

  1
• 

  P

  Mukarram Haq•440

  @MukarramHaqPosted 3 months ago

  Hey!

  Amazing solution. I looked at your code and I think your CSS is pretty well structured and easy to read.

  The CSS variables are well-defined and I don't see a problem with it. The problem usually comes with naming classes in HTML which is a headache. You can use this resource to learn more about BEM. I usually use TailwindCSS where I really don't have to worry about naming classes so that is pretty cool.

  When it comes to responsive design, I suggest start with mobile first approach. The reason being is that it is relatively easier to expand to a larger screen than to cram all of that content into a smaller screen. Trust me it will help out a lot.

  But, other than that your solutions looks amazing and good job! :)

  Marked as helpful

  0

  ---

  

  Park Byung Eun•40

  @ethan-geekPosted 3 months ago

  @MukarramHaq Thank you for the feedback! Your great advice really motivates me. I’ll proceed with future solutions following the approach you recommended.

  0