Design comparison
Solution retrospective
Getting the footer to stick to the bottom and independent of the flexbox was the biggest challenge for me. Hoping I did that right. Not sure I did the best sizing with the QR image. I'm thinking there should be a way to keep the same aspect ratio. Please comment if you have a better way...but I will look at solutions online. Also, struggled with getting it to work with mobile sizing. I used @media and build for desktop 1st. Really enjoyed doing this and anxious to get feedback, review solutions and move onto more challenges. Thanks!
Community feedback
- @denieldenPosted over 2 years ago
Hi DSoluk, great work on this challenge! 😉
Here are a few tips for improve your code:
- remove
height
fromimg and main
tags, it is not necessary because it will take the necessary space according to the size of the elements - use
h1
for the title of card and not simplep
element - remove
max-width
from body otherwise on larger screens it will not be centered - to make it look as close to the design as possible remove
outline
frommain
tag and setwidth: 20rem
- remove
height
fromhtml and body
element and addmin-height: 100vh
to body because Flexbox aligns child items to the size of the parent container - instead of using
px or %
use relative units of measurement likerem
-> read here
Overall you did well 😁
Hope this help and happy coding!
Marked as helpful0@dsolukPosted over 2 years ago@denielden Thank you for the great feedback..thanks to your efforts and well organized instructions, I was able to make the changes and was very happy with results. I definitely over complicated my solution but am better off with feedback received. Thanks again!!
On to the next challenge!
0 - remove
- @AbdelrhmanX7Posted over 2 years ago
hi DSoluk first you did a great job but there is some error i found you did it 1- in main you can remove height: 80% and use height: fit-content 2- i prefer you to use a specific instead of use percentage width because the percentage width change when the web width change example if the web width: 1000px and you use width: 30% in your element in this point your element width will be 300px it will be okay but when the screen will increase or reduce your element width will increase or reduce the web width => 1500px now your element width will be 450px and this will not be a better thing you do 3- if you want to make your element in the middle of the page you can check this article it include all ways to make an element in the middle of the page (https://blog.hubspot.com/website/center-div-css)
Marked as helpful0@dsolukPosted over 2 years ago@AbdelrhmanX7 Thanks so much for the feedback...very helpful...the whole sizing thing is a bit confusing but expect with more experience it'll settle in...again, thanks for taking the time for the feedback!!!
0 - @grace-snowPosted over 2 years ago
Hi
I'm afraid this looks totally broken for me viewing on mobile. You've already had lots of blood advice above so I won't repeat it. Just make sure you refractor code before moving on to anything else
I'll add a few more
- you're missing a heading element in the html. There is not 2 paragraphs in the design. There is a heading and paragraph
- img alt text is not code, it's a human readable description of the image. It needs to say something like "QR code for frontendMentor.io" " font sizes must never be in px. Use rem. This will allow text to scale no matter what settings a user sets on their browser or device
- this challenge should not have a media query. You don't need to change font sizes
I hope this helps
1
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