Html css/week2/berhane#1
Conversation
Rody-Kirwan
left a comment
Rody-Kirwan
left a comment
There was a problem hiding this comment.
Nice work Berhane.
I've left some notes / suggestions.
Also be sure to auto format your code before submitting. Large white spaces make things difficult to read so if you find these useful when you are working please remove them before you submit.
| .parent { | ||
| background-color: #ffffff; | ||
| margin: 0; | ||
| padding: 30px; |
There was a problem hiding this comment.
padding here is causing a weird offset.
Although I see that it seems to fix something else - maybe we'll take a look at debugging this in the session later as these things can be frustrating to find the root cause
| background-color: #ffffff; | ||
| margin: 0; | ||
| padding: 30px; | ||
| height:100%; |
There was a problem hiding this comment.
I don't think this height 100% is necessary.
Let the content dictate the height of your website :)
| display: inline-block; | ||
| margin: 1.25rem 5px; | ||
| border: 1px solid #eaebec; | ||
| padding: 0.625rem 0; |
There was a problem hiding this comment.
Some problems with the shape of these icons.
To get a circle shape the border-radius must be 50%
Also - you are adding vertical padding but no horizontal padding. I would remove the padding altogether and use flexbox to position the icons.
border-radius: 50%;
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
width: 2.5rem;
height: 2.5rem;
/* display: inline-block; */
margin: 1.25rem 5px;
border: 1px solid #eaebec;
/* padding: 0.625rem 0; */
/* text-align: center; */
| .footer .instagram{ | ||
| color: #3f729b; | ||
| } | ||
| .fa { |
There was a problem hiding this comment.
In general I would be trying to stay away from editing 3rd party classes...
Although it looks like this was done with purpose... lets chat about it later :)
|
|
||
| } | ||
| .top_content h3{ | ||
| /*padding-left: 12.5rem;*/ |
There was a problem hiding this comment.
It's good practice to remove any commented / unnecessary code before submitting a PR.
| margin-block-start: 1em; | ||
| margin-block-end: 1em; | ||
| margin-inline-start: 0px; | ||
| margin-inline-end: 0px; |
There was a problem hiding this comment.
In general - if the value is 0 we don't specify px
zero is zero :)
| margin-inline-end: 0px; | |
| margin-inline-end: 0; |
| <div class="bottom_content"> | ||
|
|
||
| <h2>Everyone needs a little karma.</h2> | ||
| <ul class="case"> |
There was a problem hiding this comment.
ul elements direct children should ALWAYS be li
You can have divs within the li but should always be
<ul>
<li>
...anything you like
</li>
<li>
...anything you like
</li>
<li>
...anything you like
</li>
</ul>
I actually think it may be this that is causing the whitespace on the right of the webpage
|
|
||
|
|
||
|
|
||
| <!--div><ul class="icons"> |
| </body> | ||
| </html> | ||
|
|
||
| </html> No newline at end of file |
There was a problem hiding this comment.
FYI
Formatting and indentation could be improved throughout this file.
Sometimes it is easier to create large spaces between elements as you are developing - but you should remove these before you submit your work as it is quite difficult to read
2 participants
final pull request