TromphiesGIG• 20
@TromhiesGIG
Submitted
i'll love to have a review, smiles ;)
Secre
@SecreSwalowtail
All comments
- Secre• 210
@SecreSwalowtail
Posted
Hello, Nice work on your QR Component.
Here are some tips to improve it :
- You can use min-height: 100vh; on the body element to make it scale to the whole screen . This will also resolve the placement issue that you talked about , but you will need to adjust the existing padding/margin on other elements.
- It's best practice to not use set height/width. Use instead min-max height/width.
- Another good practice is to use rem / em measurement units for sizing components. Design files are usually made with them not with px units.
- You can target elements like body and main by simply typing the element with no dot.
0 - Kazuha kaito• 40
@theerapat014
Submitted
I AM newborn with code. IF you have feedback plese tell me. Thank you
Secre• 210@SecreSwalowtail
Posted
Hello Kazuha ,
I have some tips that may help you :
-
Avoid using grid for the body. You can use flex to position the main card. Grid is more used when you have complex designs.
-
Avoid using fixed height/width. You can use min-height to give the card a minimum height that will always stay and a max-width to stop it from expanding more than that The content overflows to the left for this reason
-
Try and give the texts in the card a text-align to center them
-
A best practice is to only type CSS rules that you need If you type rules that at first doesn't do anything , later it may conflict with other CSS rules
-
You can look into a reset stylesheet. It removes a lot of headache from images, content not having the right padding/margin etc.
Resources :
- https://www.joshwcomeau.com/css/custom-css-reset/
- https://css-tricks.com/snippets/css/a-guide-to-flexbox/
- https://www.w3schools.com/cssref/pr_dim_min-height.php
- https://www.w3schools.com/cssref/pr_dim_min-width.php
Keep up the work , this is very nice.
0 -
- Rasmus Bremholm• 10
@Visceral89
Submitted
Feedback welcome. Not sure how to solve the padding.
Currently working on the desktop version. In mobile preview it works fine :)
Secre• 210@SecreSwalowtail
Posted
Nice work Rasmus,
There are some things you can try:
- Setting the min-height of the body to 100vh will allow the background to fill the whole screen
- Setting the display of the body to flex . This will allow you to center the card on the screen using align-items and justify-content
- You can use border-radius on the image to give it a border similar to the card
- To make the card scale nicely on higher resolutions you can set a max-width to the card. Play around with the values until you are satisfied (try 400px and go from there)
Marked as helpful
1
