-
Notifications
You must be signed in to change notification settings - Fork 110
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
Marlyn's Personal Portfolio Site #87
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.
I left a few style notes but overall everything looks pretty good.
Well done!
<h2>Background</h2> | ||
</header> | ||
<main> | ||
<div class="back-color"> |
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.
Style: back-color
isn't a good class name, since it doesn't convey any semantic meaning here. If you changed the styles for your back-color
class then you might need to rename it.
</header> | ||
<main> | ||
<div class="back-color"> | ||
<h1>Interests</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.
Style: Your heading tags h1
, h2
, etc should allow you to construct an accurate table of contents.
It looks like you used a h1
here just to make the text larger, which is better accomplished with CSS.
</nav> | ||
|
||
<header class="header"> | ||
<h2>Background</h2> |
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.
Style: Since this is the page title it should be a h1
.
<div class="my-info"> | ||
<p id="text"> | ||
Hi! My name is Marlyn Lopez. | ||
I am an aspiring software developer residing in the West Coast with a background in audio production. | ||
Please click <a id="link" href="about.html">here </a> if you're curious to know more! | ||
</p> | ||
</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.
Style: This div
is unnecessary since there's nothing else inside of main
.
In general (if you have time) it's good to do a cleanup pass to see if you can eliminate any elements that aren't meaingful like div
s.
<h2>Welcome Page | ||
</h2> | ||
</header> | ||
<div class="main-page-content"> |
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.
Style: your main page content should go inside of the main
tag.
<h6>While at Ada Developers Academy, I was able to work on a variety of projects, such as | ||
"Viewing Party", where I had to work with nested data structures to obtain data for a group of friends. "Swap Meet" was an object oriented based program where I created a program to allow groups to find certain items they preferred and swap them with each other. | ||
The most recent project, "Task-List-API" was a back end project where I used SQLAlchemy, Flask, and Python to create an API with RESTful routes for tasks and goals. | ||
I also just learned about HTML and CSS, which is how I was able to create this website! | ||
</h6> |
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.
Style: This is another place where it looks like you used a heading tag just for font sizing. Again, CSS would better accomplish this. (You could simply put a class on your article
tag and style that.
Personal Portfolio Site
Congratulations! You're submitting your assignment!
Comprehension Questions