-
Notifications
You must be signed in to change notification settings - Fork 954
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
Sharks - Philomena Kelly #29
base: main
Are you sure you want to change the base?
Conversation
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.
Nicely done, Philomena! Remember to remove any commented code you are no longer using from the final submission. I made a few suggestions about changing out a few HTML elements, but other than that this looks just fine!
</section> | ||
|
||
</main> | ||
<footer><div class="bottom">(c) philomena</div></footer> |
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.
Careful with using div
tags as a text container. They are used to divide collections of content.
Instead, let's use a p
tag or other elements that are text-specific
<meta http-equiv="X-UA-Compatible" content="IE=edge"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<title>Document</title> | ||
<meta charset="UTF-8"> |
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.
👍 no validation errors!
<meta http-equiv="X-UA-Compatible" content="IE=edge"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<title>Document</title> | ||
<meta charset="UTF-8"> |
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.
👍 no validation errors!
<div id="container"> | ||
|
||
|
||
<div class="project"><h1>project</h1></div> | ||
<div class="project"><h1>project</h1></div> | ||
<div class="project"><h1>project</h1></div> | ||
|
||
|
||
</div> |
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.
Let's make the div.container
a section
instead
Also there should only ever be one h1
per page, so perhaps h2
would be better here
</div> | ||
|
||
</main> | ||
<footer>(c) philomena</footer> |
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.
Oops, you are missing your styling class like the other pages
/* object-position: 0% 10%; */ | ||
/* filter: hue-rotate(300deg); */ |
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.
/* object-position: 0% 10%; */ | |
/* filter: hue-rotate(300deg); */ |
|
||
|
||
/* #hello { | ||
background-color: lightpink; | ||
top: 25%; | ||
left: 0px; | ||
position: absolute; | ||
height: 50%; | ||
width: 100%; | ||
text-align: center; | ||
} */ | ||
|
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.
/* #hello { | |
background-color: lightpink; | |
top: 25%; | |
left: 0px; | |
position: absolute; | |
height: 50%; | |
width: 100%; | |
text-align: center; | |
} */ |
/* nav { | ||
text-align: right; | ||
background: none; | ||
padding: none; | ||
|
||
} | ||
|
||
nav a { | ||
text-decoration: none; | ||
font-size: 20pt; | ||
font-weight: lighter; | ||
background: white; | ||
padding-left: 20px; | ||
line-height: 125%; | ||
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.
/* nav { | |
text-align: right; | |
background: none; | |
padding: none; | |
} | |
nav a { | |
text-decoration: none; | |
font-size: 20pt; | |
font-weight: lighter; | |
background: white; | |
padding-left: 20px; | |
line-height: 125%; | |
color: black; | |
} */ |
|
||
/* background-color: yellow; */ |
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.
/* background-color: yellow; */ |
.name_container { | ||
display: grid; | ||
width: 700px; | ||
/* background: grey; */ |
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.
/* background: grey; */ |
Personal Portfolio Site
Congratulations! You're submitting your assignment!
Comprehension Questions