QR-code-component-v1.0

Community feedback

• 

  Rostyslav Nazarenko•560

  @rostyslav-nazarenkoPosted almost 2 years ago

  Hi again!

  Mostly the same issues read my comment on your previous challenge.

  I noticed only one thing. Don't set width and height to card element. Right now it is completely fine because you have a fixed font size and a card with a fixed size. But later on, when you will work on bigger challenges or projects it might create problems, like overflows, text on top of another text. As a quick fix, use rem units for width and don't use height at all

  Marked as helpful

  1
• 

  Melvin Aguilar 🧑🏻‍💻•61,160

  @MelvinAguilarPosted almost 2 years ago

  Hello there 👋. Good job on completing the challenge !

  I have some suggestions about your code that might interest you.

  HTML 📄:

  • Figure elements (<figure>) should only be used when captions are required with <figcaption>, you can directly use the image tag in this solution.

  Alt text 📷:

  • The alt attribute should explain the purpose of the image. Uppon scanning the QR code, the user will be redirected to the frontendmentor.io website, so a better alt attribute would be QR code to frontendmentor.io

    If you want to learn more about the alt attribute, you can read this article. 📘.

  CSS 🎨:

  • The images have a default bottom space of around 3px, you can remove it using display: block on the image, which might be why it's not exactly the same as the Figma. Even so, it's a very similar solution to the designs.

  • The issue with the BEM naming convention in your code is that the block name (or the top-level element) is missing and the class names "qr-code__img-area" and "qr-code__text-area" do not have a block name to indicate which component or feature they belong to.

    In this case, the class name should be like this:

    qr-code
    qr-code__img-area
    qr-code__text-area
    

  I hope you find it useful! 😄 Above all, the solution you submitted is great!

  Happy coding!

  Marked as helpful

  1