-
Notifications
You must be signed in to change notification settings - Fork 119
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
Qianling - Portfolio page #71
base: main
Are you sure you want to change the base?
Conversation
…inks to the left and added button element to the right
|
||
|
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 whitespace looks odd, let's remove it :)!
|
||
<div class="about"> | ||
<p class="title"> | ||
Hey there, I'm Qianling! <br />An aspiring Software Engineer. |
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.
Personally I am not the biggest fan of <br />
. I would rather make another <p>
and use margins to create the desired distance between the lines
|
||
</p> | ||
<div> | ||
<button class="say-hello-button">Let's Start ></button> |
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.
You could try seeing if we need the <div>
without much of a use. Can we make in-line elements to block elements? :)
<h2 class="footer-header">Contact</h2> | ||
<div class="contact"> | ||
<a href="https://github.com"><img src = "github.png" alt="github-logo"></a> | ||
|
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.
whitespace
<div class="contact"> | ||
<a href="https://github.com"><img src = "github.png" alt="github-logo"></a> | ||
|
||
<a href="https://www.linkedin.com" target="blank"><img src = "linkedin.png" alt="linkedin-logo"></a> |
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.
I think we could break this into multiple lines to make it more readable
margin: 0; | ||
} | ||
|
||
/* header section */ |
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 comment is somewhat redundant as the class name is self-explanatory
font-family: "Segoe UI", Tahoma, Geneva, Verdana, sans-serif; | ||
} | ||
|
||
/* home section */ |
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.
Same here, somewhat redundant comment
background-color: whitesmoke; | ||
} | ||
|
||
/* project section */ |
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.
Here also
No description provided.