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

Semret - Personal Portfolio - Edges #31

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

Conversation

snicodimos
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? Parent and child relations in my html. I didn't fully resolve it as I didn't know how to switch it around without messing the follow of the content.
Why is it important to consider and use semantic HTML? For better readability, and organization of the content.
How did you decide to structure your CSS? I like the follow of the content and when I decided what to include on my site I decided the structure of it.
What was the most challenging piece of this assignment? Understanding the validation area and also making the timeline of projects.
Describe one area that you gained more clarity on when completing this assignment making grid and flexbox.
Optional
Did you deploy to GitHub Pages? If so, what is the URL to your website?
Overall

@droberts-sea
Copy link

Personal Portfolio Site

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage I think you could do a better job of breaking work up into small commits. When I see a message like "final 4 pages", that tells me you forgot to commit.
Answered comprehension questions yes - you might want to look up "follow" vs "flow"
Page fully loads yes
No broken links (regular or images) yes
Includes at least 4 pages and styling yes
HTML
Uses the high-level tags for organization: header, footer, main could be better
Appropriately using semantic tags: section, article, etc. yes
All images include alternate text yes
CSS
Using class and ID names in style declarations yes
Style declarations are DRY Many styles (like the banner and the nav buttons) are common to all your pages, and the CSS for these are repeated. A good approach here is to put all common styles in a separate .css file, and have each .html file link against both the common and page-specific stylesheets.
Overall Good work overall! I see some room for improvement around high-level organization, both of your HTML and your CSS. However, you were able to build an attractive, functional website, and it's clear to me that the learning goals for this assignment were met. Keep up the hard work!

<section>
<ul class="menu">
<li id="home">
<a href="index.html">Home Page</a>

Choose a reason for hiding this comment

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

Since it's used for navigation, this should probably be a <nav>, not a <section>.

<section class="sect2">
<div class="parg2">Currently, I am an Ada Developers Academy Student for C10 cohort and loving it. In the near future, I see myself working for a large tech company as a full stack developer. I want a career that is as challenging as it is exciting, but one that also makes life easier for people. Adding to the solid programming skills I will be gaining from Ada, I plan to keep the learning spirit and expand my knowledge with practical experience to develop a better understanding of all the layers of software development.
</div>
</section>

Choose a reason for hiding this comment

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

You should pick more meaningful class names than sect1 or sect2. Think of them the same way you would variable names.

</head>
<body>
<header><h2>About</h2></header>

Choose a reason for hiding this comment

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

You should break line 10 across multiple lines.

/* The typing effect */
@keyframes typing {
from { width: 0 }
to { width: 50% }

Choose a reason for hiding this comment

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

This was a cool effect - nice work getting this figured out!

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