QR-code-coding-challenge

Community feedback

• 

  Kimo Spark•2,190

  @kimodev1990Posted 11 months ago

  • It's better in the future to avoid giving your design sizes definite values such as your div with class container width: 1440px and div with class QR-content width: 280px margin-top: 140px, It's better to give percentage values to make your design flexible, for example you could assign width: 40% to class QR-content, So It's width will be 40% in relative to div class container.
  • You'd better check on clamp ( ) method ( font-size, width, etc.), so your design sizes will change according to the viewport dimensions ( Responsive Design ) and will be suitable for any device layout.

  Hope You find this Helpful.

  Other than that, Nice work & keep Going on.

  Marked as helpful

  0

  ---

  

  HendKH•10

  @HendKHPosted 10 months ago

  @kimodev1990 I liked the clamp() method !Nice idea! Thank you.I will try it in the next challenges for sure.

  0

  

  Kimo Spark•2,190

  @kimodev1990Posted 10 months ago

  @HendKH You're always welcome, Anytime...

  0
• 

  Enmanuel Otero Montano•2,155

  @Enmanuel-Otero-MontanoPosted 11 months ago

  Hello @HendKH!

  Your solution is good, but I would like to make a couple of suggestions.

  Regarding what you ask about how to optimize your code, it is difficult to make an assessment about this in this project, since it is a project that does not need much code, perhaps in a project that requires more code you can see if there is room for improvement , but I'm going to the point, something that I could find in this project is that you establish at the beginning of your code margin and padding 0 for all elements and in my opinion this is not optimal, since there are elements that have padding or margin by default , with this there is a high probability that you will have to apply margin or padding to an element that already had one of these characteristics removed at the beginning of the code.

  At this point you may think that the margin and padding of the default elements may never match the ones you want to apply, so you will always have to apply these properties to the elements, and that may be the case, but still It is not a reason to apply margin and padding 0 to all elements because if we look at how the browser interprets the code, you are requiring the browser from the beginning to set margin and padding to all elements at 0, so that it can later apply these properties again in the elements with the values ​​that you establish, this definitely in larger projects would somewhat affect the loading speed of your website, which is not good for the user experience or for positioning in search engines.

  I hope you understand what I'm trying to explain, if not, you can ask again. I forgot to tell you that I do agree with box-sizing: border-box;

  One last thing, always write the styles in a separate file to the HTML file.

  Greetings

  Marked as helpful

  0

  ---

  

  HendKH•10

  @HendKHPosted 10 months ago

  @Enmanuel-Otero-Montano thank you very much for your adviceshc regarding the padding and margin set to 0 . I thaught , by this way that I will control the padding and margin of each one of the elements apart . But it seems that is not effecient ,if I will work on a more complex project, since it will have effects on UX and SEO .I've really appreciated your advices.Thank you again.

  0
• 

  Daniel 🛸•44,310

  @danielmrz-devPosted 11 months ago

  Hello @HendKH!

  Your project looks excelent!

  I just have one minor suggestion:

  • You can replace your div.container with main.container. This won't change anything visually, but will make your project more semantic. We use semantic tags to improve the SEO optimization and accessibility of the project.

  I hope it helps!

  Other than that, you did a great job!

  0

  ---

  

  HendKH•10

  @HendKHPosted 10 months ago

  @danielmrz-dev thank you for your suggestion .I will change it .Thank you very much.

  0