-
Notifications
You must be signed in to change notification settings - Fork 91
add code again #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
add code again #67
Conversation
HIPPIEKICK
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job with this page! My main feedback is to keep the CSS to the CSS file. It will make it easier for people reviewing, but also for you when you're going back to the code and need to change something (you will not need to think - hmm, did I put this styling in the HTML or the CSS file).
Apart from that I think your code looks good (and your page too) :)
| <style> | ||
| body { | ||
| margin: 0; | ||
| } | ||
|
|
||
| div { | ||
| padding: 65px; | ||
| } | ||
|
|
||
| h1 { | ||
| text-align: center; | ||
| color: rgb(5, 39, 5); | ||
| } | ||
| h2 { | ||
| text-align: left; | ||
| color: rgb(5, 39, 5); | ||
| } | ||
|
|
||
| .top { | ||
| background-image: url(images/algues.jpeg); | ||
| /*background-color: rgb(144, 232, 144);*/ | ||
| font-style: white; | ||
| /* color: rgb(241, 245, 249);*/ | ||
| } | ||
|
|
||
| .middle { | ||
| background-color: white; | ||
| } | ||
|
|
||
| .down { | ||
| background-color: #111f11; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to keep all the CSS to the CSS file so you have it all in one place, away from the HTML 🙌
| img { | ||
| border-radius: 50%; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want an image to be truly round, you need to first make it square so that it has the same height and width. In case of distortion, you can always add object-fit: cover; to solve that.
| /* Style the button and place it in the middle of the container/image */ | ||
| .container .btn { | ||
| position: fixed; | ||
| top: 50%; | ||
| left: 50%; | ||
| transform: translate(-50%, -50%); | ||
| -ms-transform: translate(-50%, -50%); | ||
| background-color: #555; | ||
| color: white; | ||
| font-size: 16px; | ||
| border: white; | ||
| cursor: pointer; | ||
| border-radius: 50px; | ||
| padding: 20px; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! A suggestion is to move this button to a corner so it doesn't cover anything else on your page.
| .container .btn:hover { | ||
| background-color: black; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't working because you've styled the element directly in the HTML. If you remove the background-color from the HTML styling, this will work.
No description provided.