Design comparison
Community feedback
- @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. Thealt
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 helpful0 - Since this component involves scanning the QR code, the image is not a decoration, so it must have an
- @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 afooters 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: π
- β οΈ Change
width
tomax-width
in your componentβs container to make it responsive. You will also want to remove theheight
as it is unnecessary.
- Change β οΈ
width
tomax-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 helpful0 - Remove
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord