Community feedback

• 

  Abdul Khaliq 🚀•72,560

  @0xabdulkhaliqPosted 7 months ago

  Hello there 👋. Congratulations on successfully completing the challenge! 🎉

  • I have a suggestion regarding your code that I believe will be of great interest to you.

  ---

  QR iMAGE ALT TEXT 📸:

  • The QR Code Component involves scanning the QR code, the image is not a decoration, so it must have an alt attribute which should explain the purpose of the image.

  ---

  • The alt with QR Code is not even explaining for what the QR image need to be used.

  ---

  • So update the alt with meaningful text which explains like QR code to frontendmentor.io

  ---

  • Example: <img src="/images/image-qr-code.png" alt="QR code to frontendmentor.io" class="qr__img">

  ---

  .

  I hope you find this helpful 😄 Above all, the solution you submitted is great !

  Happy coding!

  1

  ---

  

  Amin Xatiashvili•40

  @AminXatiaPosted 7 months ago

  Tnx for your kind words and the point you made bro 🤍 @0xabdulkhalid

  0
• 

  Sumedha Koranga•70

  @sumedhakorangaPosted 7 months ago

  This is so close, Nice.

  1
• 

  Daniel 🛸•44,310

  @danielmrz-devPosted 7 months ago

  Hello @AminXatia!

  Your solution looks great!

  I have a suggestion for improvement:

  📌 Think about using <main> to wrap your main content instead of <div>.

  Imagine <div> and <span> in HTML as basic containers. They're good for holding stuff, but they don't tell us much about what's inside or its purpose on the webpage.

  This change might not have impact on how your page looks, but it'll make your HTML code clearer and help with SEO and accessibility.

  Hope that's helpful!

  Keep up the great work!

  0

  ---

  

  Amin Xatiashvili•40

  @AminXatiaPosted 7 months ago

  Hey, Daniel 👋. Thank you for leaving this comment and giving your advice. I try to keep them in mind@danielmrz-dev

  0
• 

  P

  Jeff Guleserian•480

  @jguleserianPosted 7 months ago

  Greetings, Amin,

  Congradulations on your solution to the QR Code challenge! Very well done! I was impressed with the similitud to the prototype.

  Other comments:

  • Your HTML and CSS is easy to read and follow. It looks like you used BEM for organizing your CSS. While this is not my personal favorite, it is a system widely used by a lot of teams and instructors - so cudos to you for implementing it so early in your submissions. I do agree with Bernardo (above) however, that you could use more semantic HTML.
  • Your solution technically correct and also uses at least one accessibility practices. One practice you might want to add is a <meta> tag in the head that contains description of the page, something like this: `<meta name="description" content="[description of page]">. I recommend this because it is also a good practice for search engine optimization (SEO). There are a lot of other practices that I'm sure you will want to investigate later if you have not already done so.
  • I noticed that you did not have any @media statements for the smaller screen size. If I remember correctly, there did exist some minor adjustments for the smaller screen size (like width/height), thought it's so forgiveable in this case since the directions do not specify anything and the changes are so subtle.
  • I would like to encourage you to complete the README.md file and keep it with the code. This will help future reviewers understand your intention and workflows. It also may help them provide you with better feedback. If you need help writing markdown (though you can probably guess it from just using the template), I recommend the Markdown Guide. They have a nifty cheat sheet that covers all the basics and more.

  Anyway, Amin, I want to tell you again that I liked your solution and think you did an outstanding job. I hope you find something in my comments helpful. I also wish you all the best as you continue your professional development.

  Happy coding!

  Jeff Guleserian

  0

  ---

  

  Amin Xatiashvili•40

  @AminXatiaPosted 7 months ago

  I try to pay more attention to what you said and try to apply them in future projects and files. Thank you for helping me with your useful tips sir 🌹@jguleserian

  0
• 

  Bernardo Poggioni•5,360

  @R3ygoskiPosted 7 months ago

  Hello @AminXatia, I would like to congratulate you on completing your first challenge from Frontend Mentor, and you started off very well because the solution looks identical to the proposed design, congratulations.

  I would like to give you some tips about your HTML, it is well structured, but it is not semantic, and being semantic is extremely important for the accessibility of a page. I will show you a section that you could make semantic.

  • <div class="container"> instead of using a <div> here, you could use a <main> because this is where the main content is.

  Another suggestion regarding HTML, I noticed that you used something similar to BEM, right? About this, I noticed that there are many wrappers in your HTML, a suggestion would be to change them to other names. And we usually use wrapper for just one element.

  For example: div class="wrapper"> it would make more sense if it were a container instead of wrapper.

  And again, congratulations on your project, it is very well done, keep practicing and improving, you are doing great. If anything I said is confusing, please comment below and I will try to help as best as possible.

  0

  ---

  

  Amin Xatiashvili•40

  @AminXatiaPosted 7 months ago

  Thank you for leaving this great comment. I found it really useful and I try to keep them in mind from now on. Tnx 🤍@R3ygoski

  0

  

  Bernardo Poggioni•5,360

  @R3ygoskiPosted 7 months ago

  Hi @AminXatia, no problem, always happy in help others devs, if you found my comment valuable and it helped you, I kindly ask that you please mark it as helpful, as it helps me a lot. Thank you in advance!

  Marked as helpful

  0