-
Notifications
You must be signed in to change notification settings - Fork 155
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
Personal Portfolio - Jennifer N. #119
base: master
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.
Great job! You got a lot of mileage out of your markup and styling. Everything looks really clean (especially love the about page), with maybe the only rough edgebeing the list of technologies on the home page. But overall, very effective.
@@ -1,12 +1,46 @@ | |||
<!DOCTYPE html> |
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.
Prefer not to nest the images folder within the pages folder.
<header> | ||
<h1 class="page-title"> About Jennifer </h1> | ||
<nav class="nav-menu"> | ||
<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.
We can do without the div
s here. Since the div
s just wrap the a
s directly, making the a
s the direct children of the nav
would have the same effect (since the nav
has grid
styling).
Alternatively, since we should aim for making our markup look good even if the styles are absent, we might logically group the a
s under a list. It's not necessary when the styles are applied, but can be handy from a logical standpoint.
<h2>Glowing Reviews</h2> | ||
<div> | ||
<p class = "stars"> ⭐⭐⭐⭐⭐ </p> | ||
<blockquote>"Amazing to work with! 5 stars."</blockquote> |
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.
Love these quotes! 😁
<a href="/pages/index.html"> Home </a> | ||
</div> | ||
<div> | ||
<a href="/pages/portfolio.html"> Portfolio </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.
Be careful using absolute paths for links/resources. They can simplify refering to files within your site, but they make the assumption that this page will be hosted at the root location of website, which isn't always true for all deployment hosts.
|
||
<!-- portfolio --> | ||
<main class="portfolio"> | ||
<div id="project_1"> |
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.
Consider using a section
for each of these, since they have a heading and additional content.
@@ -0,0 +1,69 @@ | |||
# This file must be used with "source bin/activate" *from bash* |
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 need to create a venv since this isn't a python project.
<main class="hero-page-content"> | ||
<h2>Hi, I'm Jennifer</h2> | ||
<p> and I write <code>#code</code> in:</p> | ||
<ul> |
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 might style this to remove the bullets and left margin. That would logically keep these a lit, but visually center them along with the rest of the content.
Personal Portfolio Site
Congratulations! You're submitting your assignment!
Comprehension Questions