Mary Nabwire• 40
@Mary83-ops
Submitted
safdar
@sfdrk
All comments
- safdar• 130
@sfdrk
Posted
Hi 👋 I think you could add margin for the main container .... so it will have the space on responsive . Just see in mobile screen it is enlarging to the edge of the screen.
Marked as helpful
0 - 'Tiana Coker• 40
@Tiana-Coker
Submitted
safdar• 130@sfdrk
Posted
Hey 👋 Nice work ... Great design .... Here is my suggestions
You can remove the padding of body completely .... then you can add min-height:100vh for the body .... You can center the main container in different ways .... what i suggest is flex and grid . Here are the styles .
body{min-height:100vh;display:grid;place-items:center}
or
body{min-height:100vh;display:grid;align-items:center;justify-content:center}
Then you can add max-width for your main-container because then it will not go out of flow it will stay relatively to your screen sizes ... that's good for better responsiveness.
main-container{max-width:350px;width:100%}
Your design is pretty good 🎉
Marked as helpful
1 @Abanoub-Ashraf1
Submitted
All Feedback is Welcome
safdar• 130@sfdrk
Posted
Hey 👋 You can add max-width instead of width for your container element ... then it will not go out of flow .... it will stay relatively to your screen size , i mean for better responsiveness.
.container{ max-width:300px; width:100%; }
0- amirbek887• 30
@amirbek887
Submitted
safdar• 130@sfdrk
Posted
Hey 👋 These are my suggestions ... To center .qr-code you can add min-height:100vh; display: flex; align-items: center: justify-content: center; or min-height:100vh; display: grid place-items: center to the body. The style will become like this
body{ min-height:100vh; display:grid; place-items:center;}
or
body{ min-height:100vh; display:flex; justify-content:center; align-items:center; }
margin-left and margin-right auto is ok when you want to center horizontally.
Then instead of only width you can add max-width for qr-code block wont go out of flow it will stay relatively to your screen size .. try adding that and check for the difference.The style will be like this
qr-code{ max-width:220px; width:100%; }
You also didn't add proper given font-family you can find fonts on google fonts.They are already mentioned too. (https://fonts.google.com/)
😊
1 - Carolina Rangel• 10
@carolrangel98
Submitted
Hello, Frontend Mentor community! Here's my approach to the challenge's final solution. Accepting any suggestions, tips and observations.
safdar• 130@sfdrk
Posted
Hey 👋 .... I'm just adding my suggestions over above .... you can add max-width for qr-box instead of width , like below
qr-box{ max-width:225px; width:100%; }
So it will fit responsively for the screen size .... the qr box width wont go out of flow .. You can try adding and see the difference !!!
Marked as helpful
1 - generieyyc• 160
@generieyyc
Submitted
safdar• 130@sfdrk
Posted
am just adding that ... always try to make image styles as :-
img{ max-width:100%; width:100%; height:auto; }
So it will resize relative to your screen sizes. I mean for better Responsiveness
Marked as helpful
0 - João Portes• 10
@JoaoLuisPortes
Submitted
Sou iniciante e ficaria muito feliz se você deixasse seu feedback para me ajudar a melhorar.
Qualquer crítica é bem vinda. 😉
I'm a beginner and I would be very happy if you leave your feedback to help me improve.
Any criticism is welcome. 😉
safdar• 130@sfdrk
Posted
make your qr box height as auto .... you are given 520px height , bcs of that text is going out of flow.
Marked as helpful
0 @zousk-Camilo
Submitted
my first web with sass, without help, any feedback is welcome, thanks
safdar• 130@sfdrk
Posted
Looks good .... you could have add transition for button , that would be smooth when hover
1
