Design comparison
SolutionDesign
Solution retrospective
What type of changes should I make to make it look more cool and responsive
Community feedback
- @MelvinAguilarPosted over 1 year ago
Hello there π. Good job on completing the challenge !
I have some suggestions about your code that might interest you.
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 π¨:
- The height in the image is not necessary, you are just elongating the image for no reason. Also, the height of the component is not necessary, the content and padding should be responsible for the height. As a last option, it is better to use
min-height
instead ofheight
.
- The
width: 100vw
property in the.class="container"
tag is not necessary. Thediv
tag is a block element and it will take the full width of the page by default.
- For a responsive component, consider using
max-width
for the width and addingmargin
to add some space for the container card and the screen edge on mobile devices.
- Use
min-height: 100vh
instead ofheight
. Setting the height to 100vh may result in the component being cut off on smaller screens, such as a mobile phone in landscape orientation.
- You should use the
box-sizing: border-box
property to make thewidth
andheight
properties include the padding and border of the element. This will make it easier to calculate the size of an element. You can read more about this here π.
I hope you find it useful! π
Happy coding!
Marked as helpful2 - Since this component involves scanning the QR code, the image is not a decoration, so it must have an
- @0xabdulkhaliqPosted over 1 year ago
Hello there π. Congratulations on successfully completing the challenge! π
- I have other recommendations regarding your code that I believe will be of great interest to you.
HTML π·οΈ:
- This solution generates accessibility error reports due to
non-semantic
markup, which lack landmark for a webpage
- So fix it by replacing the element
<div class="container">
the with semantic element<main>
in yourindex.html
file to improves accessibility and organization of your page.
- What is meant by landmark ?, They used to define major sections of your page instead of relying on generic elements like
<div>
or<span>
- They convey the structure of your page. For example, the
<main>
element should include all content directly related to the page's main idea, so there should only be one per page
I hope you find this helpful π Above all, the solution you submitted is great !
Happy coding!
Marked as helpful0
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