Latest comments
- @GustavoSouza123Submitted about 1 year ago@silkcoderPosted about 1 year ago
Congrats for completing the task.
I noticed one thing, You applied the style
height:100vh
to the container element. Its good butmin-height:100vh
is more suitable here.To see why, open your site on google chrome, then open developer tools, then click on mobile view, then choose Iphone 12 pro, the site looks good but if you change the orientation, you will see some white colored space at bottom. You can fix this by applying
min-height:100vh
instead ofheight:100vh
to the element with class 'contianer'.Good luck with your future tasks.
Marked as helpful1 - @mohamedqabbariSubmitted about 1 year ago@silkcoderPosted about 1 year ago
Congrats for completing the task.
I noticed one thing, you applied flex properties to the HTML tag to center the 'card' class element.I think you should apply it to the body tag rather than applying to the HTML tag.
It's generally considered a good practice to put styles in the <body> element
1 - @Trishna-JSubmitted about 1 year ago@silkcoderPosted about 1 year ago
Good attempt.
You can place your container in the center of the screen vertically by changing the height property of the .container class
change min height:600px to min-height:100vh
.container { min-height:100vh }
Now the container get placed vertically centered on the screen.
Good luck.
0 - @Trishna-JSubmitted about 1 year ago@silkcoderPosted about 1 year ago
Hello there,
Your solution looks very good, I have a few suggestions.
The container is not centered vertically, to place the container vertically center, just add the following style to your .container class
.container { //add this property to your existing styles min-height: 100vh; }
Now you can remove margin-top property from .box class
.box { //remove this style margin-top: 50px; }
I checeked the repo and I think its good to place your assets in separate folders, like images goes to 'images' folder and css files to you a 'css' folder.
It is not a good practice to add inline styles to an HTML element (you added inline styles to a div with class box)
Good luck with your future tasks
Good day
0 - @Ezekiel225Submitted over 1 year ago@silkcoderPosted about 1 year ago
Congrats for completing the task.
I have two points to make this layout better.
I am using a wide monitor and the background image looks broken. You can make it better by adding following styles to your body tag
body { //your exisitng styles background-repeat: repeat-x; }
Once you add this styles, the background image will cover the entire screen.
When I open your page, I can see a scroll bar in the layout, its because body tag has default margin. As a quick fix, you can add margin:0px to your body tag
body { //Your existing styles margin:0px; }
Now you will see the layout without a scroll bar
Its recommended to have a reset.css file in the project to reset all the default styles. You can find sample reset file at https://meyerweb.com/eric/tools/css/reset/
Good luck with your future tasks
0 - @ghost237-sysSubmitted about 1 year ago@silkcoderPosted about 1 year ago
Hello there,
Congrats for completing your task, It looks good, however I have a few suggestions that can make your layout better
First thing you have to keep in mind is that your layout get viewed in different sized devices. so is not a wise decision to set width for body tag. so you can remove width property from body tag
body { //remove this width: 1440px }
Next, you can remove margin property you give to container div (we have a better method to center the container :) )
.container { //remove following line margin: 200px 250px 150px 580px; }
Now the container get aliged to top left, that is not good :) we have to place the container centered in all sized devices. so apply the following propertiese to the body tag in addition to the existing styles
body { //your styles here display: flex; justify-content: center; align-items: center; min-height: 100vh; }
Now the container get aligned to centered horzontally and vertically. Good job
you can read more about flex box propertise at https://www.geeksforgeeks.org/introduction-to-css-flexbox/
Now if you notice, you may see a scroll bar in the window, its because there is a margin property get added to body tag by default. so a recommended method to fix this is to use a reset css file
[this is a sample reset css file] (https://meyerweb.com/eric/tools/css/reset/)
you can download and load this css before loading your styles.css, this file reset all default styles get added to the HTML document
As a quick fix (without using a reset file) you can add 'margin:0px' to your body tag to remove the scroll bar
body { //all your styles in body tag goes here margin:0px; }
Now your layout look better.
I noticed that on your repo, all files (images, css) are scattered, Its good to keep them on separate folders, so that all your images will stay on images folder and your styles to a styles folder
Good luck with y our future tasks
Good day
0