-
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
jae c17 sea turtles #30
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.
Looks good!
Your site is really clean, both visually and mark-up-wise. I'd suggest trying to deploy again, either with changing your paths to use relative locations, or doing a non-project pages site to try to get that working.
Well done!
@@ -0,0 +1,3 @@ | |||
{ | |||
"liveServer.settings.port": 5501 |
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 must have already had another instance of live server running so it picked a different port.
@@ -0,0 +1,23 @@ | |||
#box{ |
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 don't see this being used in the site. Consider removing it.
@@ -0,0 +1,34 @@ | |||
.container { |
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 don't see this being used in the site. Consider removing it.
@@ -4,9 +4,34 @@ | |||
<meta charset="UTF-8"> | |||
<meta http-equiv="X-UA-Compatible" content="IE=edge"> | |||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | |||
<title>Document</title> | |||
<title>index</title> | |||
<link href="/style/about.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.
Absolute paths can work well if your pages are in a variety of directories and you don't want to update copy/pasted code for styles/links. But we do need to make sure the root of our deployed server will match. There are two different ways to deploy to github pages. You would want to follow the User/Org instructions here instead of the project site deployment approach. Otherwise, you may want to use relative paths.
<h1>jae clayton</h1> | ||
<nav> | ||
<ul> | ||
<li><a href="/pages/index.html">home</a></li> |
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.
Anchor links would have the same absolute/relative considerations as the style link.
<h2>jae clayton: about me</h2> | ||
<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Id volutpat lacus laoreet non. Magna etiam tempor orci eu lobortis. Morbi tristique senectus et netus et malesuada. Quis blandit turpis cursus in hac habitasse platea dictumst. Donec enim diam vulputate ut. Quis varius quam quisque id diam. Ultricies tristique nulla aliquet enim tortor at auctor urna. Commodo viverra maecenas accumsan lacus vel facilisis volutpat est velit. Dolor sit amet consectetur adipiscing elit ut. Ut pharetra sit amet aliquam id diam maecenas ultricies mi. Amet porttitor eget dolor morbi non. Consequat mauris nunc congue nisi vitae suscipit tellus mauris. Ullamcorper dignissim cras tincidunt lobortis feugiat. Duis ut diam quam nulla porttitor massa id. Ultrices eros in cursus turpis. Commodo ullamcorper a lacus vestibulum sed. Tincidunt eget nullam non nisi est sit amet. Erat imperdiet sed euismod nisi porta. Elit pellentesque habitant morbi tristique senectus et netus et.</p> | ||
</section> | ||
<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.
There was a validation warning here about not having a heading in this section. I would say the section itself isn't that necessary here. Think of section as being something that lets us express the logical structure of the content, such that if we gathered all the section headings, we'd be able to build a table of contents. Since this section doesn't really carry any semantic value, I'd replace it with a div, or see if I could remove it entirely.
<main> | ||
<section> | ||
<div class='container'> | ||
<a href="https://github.com/jaekerrclayton/swap-meet"><img src="/images/ezgif-4-cbe5863d24.gif" alt='swap-meet-photo'/></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.
From a usability standpoint, only linking the image is a little surprising. My inclination is to click the text, but the text isn't linked, and when positioned over the image, blocks the ability to click through to the image.
One thing we can do to check that thinks are laid out for usability is the temporary comment out our stylesheet link to see how the page renders without the rules, and make sure that the page is still usable.
color:#ebecd1; ; | ||
justify-content:space-evenly; |
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.
We should make sure our CSS follows consistent formatting. VSCode can help by using the "Format Document" option in the command palette.
width: 100%; | ||
height: 2.5rem; | ||
grid-column: 1/ span7; | ||
grid-row: 3/ span 4; |
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.
There are only 3 rows, so this should be
grid-row: 3 / span 1;
display:block; | ||
} | ||
|
||
.container { |
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 like having a class like this on some root element of related tags, and then using that as part of a descendant selector as you did later.
Personal Portfolio Site
Congratulations! You're submitting your assignment!
Comprehension Questions