QR code component

Community feedback

• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted almost 2 years ago

  Hello there 👋. Good job on completing the challenge !

  I have some feedback for you if you want to improve your code.

  HTML 📄:

  • Since this component involves scanning the QR code, the image is not a decoration, so it must have an alt attribute. The alt attribute should explain its purpose. e.g. QR code to frontendmentor.io

  CSS 🎨:

  • Using too many selectors or too many level nesting in SASS can make your code more difficult to read and maintain. Additionally, using too many selectors can lead to specificity issues, making it difficult to override existing styles.

    .wrapper .card-container .content .paragraph { . . . }
    

    You can use BEM naming convention to keep only one level of nesting in SASS. BEM (Block Element Modifier) is a popular naming convention for CSS classes that helps developers better organize their code. It is especially useful when working with SASS because it allows developers to create more modular and reusable code. BEM also makes it easier to maintain and debug code, as well as making it easier for other developers to understand the structure of the code.

    You can read more about this here: BEM and SASS a perfect match

  • Centering an element with position: absolute would make your element behave strangely on some screen sizes, "there's a chance the content will grow to overflow the parent". You can use Flexbox or Grid to center your element. You can read more about centering in CSS here 📘.

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

  Happy coding!

  Marked as helpful

  0
• 

  バレンタイン 😈•64,170

  @VCaramesPosted almost 2 years ago

  Hey there! 👋 Here are some suggestions to help improve your code:

  • Remove <div class="wrapper"> as it is unnecessary ❌.

  • The attribution should be wrapped in a footers instead of a div` for improved semantics.

  • The alt tag description for the “QR image” needs to be improved upon ⚠️. Its needs to tell screen reader users what it is and where it will take them to when they scan it.

  • To properly center ✅ your content to your page, you will want to add the following to your body (this method uses CSS Grid):

  body {
      min-height: 100vh;
      display: grid;
      place-content: center;
  }
  

  More Info: 📚

  Centering in CSS

  • ⚠️ Change width to max-width in your component’s container to make it responsive. You will also want to remove the height as it is unnecessary.

  • Change ⚠️ width to max-width: 100% in your image to make it responsive.

  If you have any questions or need further clarification, feel free to reach out to me.

  Happy Coding! 🎆🎊🪅

  Marked as helpful

  0