-
Notifications
You must be signed in to change notification settings - Fork 36
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
Queues - Marisol Lopez - Static Site #46
base: master
Are you sure you want to change the base?
Conversation
Static SiteWhat We're Looking For
Great work overall. Site is clean and easy to navigate, and HTML is well-structured and easy to read. I like how well-commented your CSS is - very easy to tell what's affecting what. As your CSS continues to grow more complex, it might make sense to split out the CSS for different pages into different files, so that each page can link only the styles it needs. So the project setup might look like
and then in the individual HTML files: <!-- hobby-blog.html -->
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Hobby Blog</title>
<link href="stylesheets/style.css" rel="stylesheet">
<link href="stylesheets/hobby-blog.css" rel="stylesheet">
<!-- ... --> |
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.
Great job! Your site looks nice and is easy to navigate. Good use of semantic tags and unordered list when needed. Overall Dry CSS and well named and commented. It's easy to understand what each one is doing. Keep up the good work.
Some comments:
• When you resize the window some of the texts overlap. Can you think of a way to avoid that?
• Can you think of places to use id selector?
Great job again!
<p>Hodor hodor HODOR! Hodor hodor... Hodor hodor hodor. Hodor hodor HODOR! Hodor hodor, hodor. Hodor hodor - hodor - hodor hodor! Hodor hodor - hodor; hodor hodor hodor? Hodor hodor HODOR! Hodor HODOR hodor, hodor hodor? Hodor hodor HODOR! Hodor hodor... Hodor hodor hodor. Hodor hodor HODOR! Hodor hodor, hodor. Hodor hodor - hodor - hodor hodor! Hodor hodor - hodor; hodor hodor hodor? Hodor hodor HODOR! Hodor HODOR hodor, hodor hodor? Hodor hodor HODOR! Hodor hodor... Hodor hodor hodor. Hodor hodor HODOR! Hodor hodor, hodor. Hodor hodor - hodor - hodor hodor! Hodor hodor - hodor; hodor hodor hodor? Hodor hodor HODOR! Hodor HODOR hodor, hodor hodor?</p> | ||
</div> | ||
</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.
Look out for extra white spaces.
|
||
|
||
</main> | ||
|
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.
Is the use of section appropriate here? I might also use a element to wrap the copyright. (https://developers.facebook.com/docs/instant-articles/reference/footer)
@@ -0,0 +1,137 @@ | |||
* { |
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 use of universal selector!
font-size: 40px; | ||
font-family: "Arial Narrow", Arial, sans-serif; | ||
text-align: center; | ||
padding-top: 100px; |
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.
Could you have written this(paddings) in one line instead? (https://www.w3schools.com/css/tryit.asp?filename=trycss_padding_shorthand)
color: black; | ||
} | ||
|
||
.home-page a:hover { |
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.
Great use of hover! The color pink is used in few places. You might be able to to dry it.
font-family: "Helvetica Neue"; | ||
} | ||
|
||
/*****Navigation bar at top of page*****/ |
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 on separating code by including comments!
margin-right: 5%; | ||
/*border: solid;*/ | ||
margin-left: 17%; | ||
/*float: 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.
Unused CSS be sure to remove them :).
<section class ="main-section"> | ||
<h2>PROJECTS</h2> | ||
<div class="project-images"> | ||
<a href="https://github.com/marisol-lopez/Random-Menu"><img src="../images/menu.jpg"></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.
Don't forget alt.
</head> | ||
<body> | ||
<header> | ||
<h1><a class="home-button" href="index.html"> Marisol Lopez<br> </a></h1> |
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.
Look out for spaces between a tags.
<link href="../stylesheets/normalize.css" rel="stylesheet"> | ||
</head> | ||
<body> | ||
<header> |
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 use of header! :)
Static Site
Congratulations! You're submitting your assignment!
Comprehension Questions