@bccpadge
Posted
Hello @jkaps9. Congratulations on completing this challenge!!!π
I have a few tips to might be interested in to improve your solution.
HTML π:
- When writing HTML keep in mind, a
div
is non-semantic element which tells nothing about its content and ever website should have at least one landmark - Attribution info should be wrapped in a
<footer>
tag - Change the
<h2>
to a<h1>
because it is the only title on the website
More infoπ:
CSS π¨:
- Font size must be in rem units and not pixels
- Adding
max-width: 320px;
on your card makes it responsive without using any media queries - Applying
width
orheight
CSS properties give elements a fixed width or height and doesn't change when go from mobile to desktop screens.
Instead of using flexbox, you can also use CSS Grid and saves you a couple lines of CSS as well.
.container{
display:grid;
place-content: center;
min-height: 100vh;
margin: 0 1rem; /* Adds space on left and right side on mobile screens */
}
More infoπ:
To make your image responsive add max-width: 100%
;
Here is my solution to this challenge - QR code component
I hope you find this useful and don't hesitate to reach out if you have any questions.
Marked as helpful
@jkaps9
Posted
@bccpadge Thank you so much for your detailed feedback! I have incorporated much of what you suggested above and it has certainly improved the code overall. More importantly, I have learned so much just from this one piece of feedback. I know about CSS Grid but wanted to try Flexbox for my first project on here.