Skip to content
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

Sherman, personal portfolio #45

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Sherman, personal portfolio #45

wants to merge 6 commits into from

Conversation

Peacegypsy
Copy link

Personal Portfolio Site

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Did you have to resolve any issues when running the HTML Validator? If so, what were they? incorrect links and some typos (misplaced or missing commas, unclosed parentheses ,etc.
Why is it important to consider and use semantic HTML? to be able to style tags properly and to better relay to others what you have set up(clear communication)
How did you decide to structure your CSS? It depended on the content mostly, but also I tried to utilize things we had covered in class while keeping a similar theme across all pages.
What was the most challenging piece of this assignment? coming up with the content idea and layout, followed by the css part.
Describe one area that you gained more clarity on when completing this assignment using class in tags for better and more accurate styling.
Optional
Did you deploy to GitHub Pages? If so, what is the URL to your website?

@dHelmgren
Copy link

Personal Portfolio Site

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage I would like to see much more frequent commits. It can be challenging to build this into your workflow when starting with a new language or style of programming, but that's also when it's most useful to have a record of what you did and how you did it.
Answered comprehension questions Just want to remind you that semantic html is also useful to folks who use things like screen readers.
Page fully loads See comments, multiple broken images due to absolute paths.
No broken links (regular or images) Your links are working correctly for me but your images are not.
Includes at least 4 pages and styling Yes, though you have 2 commented out stylesheets.
HTML
Uses the high-level tags for organization: header, footer, main No, see comments
Appropriately using semantic tags: section, article, etc. I don't see any semantic tags used in this assignment.
All images include alternate text Yes!
CSS
Using class and ID names in style declarations I don't see you using IDs anywhere for style.
Style declarations are DRY No, you've got several repeat CSS selections, see my comment.
Overall There were a lot of key steps that were missed, and a lot of small indications that learning around html and CSS was missed. From most concerning to least concerning, here's what I see: No semantic html, most images use absolute paths, extensive use of div tags, CSS that is not DRY. In the future when you are using html for your projects, we want to see these problems addressed. If you need help adjusting your html or have any general questions, please reach out.

<p class="flex-container">
<img src="/Users/xtina206/ada/week-six/personal-portfolio-site/images/bunny.gif" alt="Positive Bunny"/>
<img src="/Users/xtina206/ada/week-six/personal-portfolio-site/images/Grateful.jpg" alt="Grateful Rainbow" />
<img src="/Users/xtina206/ada/week-six/personal-portfolio-site/images/bunny2.gif" alt="PositiveBunny2"/>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your images don't work on any computer besides your own because you've used absolute paths for each of them. I don't have a Users/xtina206 folder on my mac, so I can't see any of these beautiful images rendering in the page.

<h2>Welcome to here</h2>


<img src="http://placekitten.com/600/300" alt="placekitten" >

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This image that you pulled from the web is the only one I could see, since it was loading from the web.




<div class="footer">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a footer tag that we went over explicitly, I'm not sure why you need a div here instead.

text-align: center;
font-family: 'Sawarabi Mincho', sans-serif;
}
.footer {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You had this class style in more than one file. Seems like a good use case for a main.css that gives default styles across the site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants