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

Shelan's portfolio #21

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

Shelan's portfolio #21

wants to merge 4 commits into from

Conversation

sheland
Copy link

@sheland sheland commented Sep 17, 2018

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? I had errors stating "element li not allowed as child of element in this context." I resolved them but not all of them because it didn't negatively impact the layout of my portfolio.
Why is it important to consider and use semantic HTML? It lets users know the contexts of your file by simply looking at the tags. It aids communication by being clear of the meaning of the page.
How did you decide to structure your CSS? I used css grid and flex-box. I probably heavily relied more on flex-box because I found myself playing with the margins/padding longer than expected.
What was the most challenging piece of this assignment? Adding links and images, I had a hard time establishing the proper pathway. Adding spacing between flex items and adding animation.
Describe one area that you gained more clarity on when completing this assignment I believe I have a greater understanding of the funflex-box, I really enjoyed manipulating the spacing of items even though at times it got a bit tricky.
Optional
Did you deploy to GitHub Pages? If so, what is the URL to your website?
Overall

@CheezItMan
Copy link

Personal Portfolio Site

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage Very few commits and not very descriptive commit messages.
Answered comprehension questions Check
Page fully loads Check, with error
No broken links (regular or images) Check
Includes at least 4 pages and styling Check
HTML
Uses the high-level tags for organization: header, footer, main Yes
Appropriately using semantic tags: section, article, etc. Check, although I would use header instead of having a section with a class set to header
All images include alternate text Check
CSS
Using class and ID names in style declarations Check
Style declarations are DRY Could be DRYer
Overall The pages work, but you have some invalid HTML and CSS. You also could make the CSS more DRY. Overall not bad and you hit the majority of the learning goals for the project.

@@ -0,0 +1,33 @@
<!DOCTYPE html>

Choose a reason for hiding this comment

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

Did you export this file from a 3rd party program? Why is (port) in the filename.

@@ -0,0 +1,21 @@
<section class="blogs">

Choose a reason for hiding this comment

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

Note: This file isn't valid HTML.

<body>

<section class="header">
<li><a href="portfolio.html">portfolio.</a></li>

Choose a reason for hiding this comment

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

li elements must be children of ul or ol elements.

<footer>
<ul class="footer">
<li>© 2018-</li>
<li>contact information: shelandebesai@gmail.com <a href="shelandebesai@gmail.com">

Choose a reason for hiding this comment

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

This li element isn't closed.

Also the a element isn't closed

<body class="container">

<section class="header">
<li><a href="portfolio.html">portfolio.</a></li>

Choose a reason for hiding this comment

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

Also li can't be a child of section, it can only be a child element of ul or ol.

color:#eee;
}

.background:before{

Choose a reason for hiding this comment

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

Nice parallax effect.

background-image: url("https://i.imgur.com/LSpa1mM.jpg");
}

.container {

Choose a reason for hiding this comment

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

Notice you have the this style repeated in multiple files. Not exactly DRY.

grid-template-rows: 1fr 1fr 4fr 1fr
}

.header {

Choose a reason for hiding this comment

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

You also have a lot of these styles repeated in multiple files

}

body {
background-image: url("https://i.imgur.com/LSpa1mM.jpg");

Choose a reason for hiding this comment

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

in CSS you use single quotes, not double.

*/

button,
[type="button"],

Choose a reason for hiding this comment

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

In CSS use single quotes, not double quotes.

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