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

Ashton's Static Site #37

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

Ashton's Static Site #37

wants to merge 9 commits into from

Conversation

ashtn
Copy link

@ashtn ashtn commented Mar 20, 2017

Static 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? Yes, I received Parse Error. Style sheets should not include HTML syntax. <!doctype html> but wasn't really clear why
Why is it important to consider and use semantic HTML? Semantic HTML is important to help provide more context to your content
How did you decide to structure your CSS? Sections that were site specific styles, and then specifics sections that were page specific
What was the most challenging piece of this assignment? Everything... But seriously, expecting the css to do one thing and it doing literally the exact wrong thing and tinkering around to find out why/how to fix it. And not knowing how to ask google the right questions.
Describe one area that you gained more clarity on when completing this assignment absolute positioning, and how it only works the way you want when you define borders (not literal element borders but you could) but setting the parent to position relative would make the child absolute to the parent as opposed to the body.

@CheezItMan
Copy link

Static Site

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage Regular commits and decent commit messages, nice!
Answered comprehension questions Stylesheets should only include css code, not html. Like in Ruby it's important to separate concerns. Plus if the file ends with .css the browser's going to get confused if the doctype says it's html.
Page fully loads Check
No broken links (regular or images) Your email link at the bottom is broken.
Includes at least 4 pages and styling 5 pages and styling.
HTML
Uses the high-level tags for organization: header, footer, main Very well structured HTML
Appropriately using semantic tags: section, article, etc. Good use of semantic HTML!
All images include alternate text Check
CSS
Using class and ID names in style declarations Check
Style declarations are DRY There some minor drying that could be done. You're reusing some colors and text styling that could be combined. However it's pretty minor.
Overall This is a very nice site. It looks great! I hope you get it published in github.io

<body>

<header>
<img id="logo" src="../Assets/static-logo.png" alt="Ashtn">

Choose a reason for hiding this comment

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

The "title" attribute is what shows up when you hover over something - that should say "Ashtn." The "alt" attribute should be as simple as possible for a screen reader - it won't necessarily know how to pronounce "Ashtn" without the "o", so I recommend changing this to "Ashton."

Choose a reason for hiding this comment

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

Regardless, good use of alt tags - you'd be amazed how many folks forget or neglect to include accessibility in their builds.

<meta charset="utf-8">
<title>Ashtn</title>
<link rel="stylesheet" href="../Styles/normalize.css">
<link rel="stylesheet" href="../Styles/styles.css">

Choose a reason for hiding this comment

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

This advice will come in handy later when you're using a preloaded framework like Foundation or Bootstrap, but there's often a "reset.css" file that's necessary to include in order to remove weird shit the browser likes to pull and give you a truly blank slate re: styling. Foundation 6 (iirc) does it for you automatically, but it's worth checking on depending on the version and framework you're specifically using.

Call reset.css first and then move on down the line. I'm not sure if normalize is the equivalent of reset here or what. This is why I'm a back-end dev XD

@@ -0,0 +1,68 @@
<!doctype html>

Choose a reason for hiding this comment

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

Side note - when it comes to social media, go for .svg files. They're scalable and you can crack open their core code and change the color to whatever you want :D


</main>

<footer>

Choose a reason for hiding this comment

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

Okay re: social media links. First quick thing - there's a way to note if your links to things like google etc open in another browser tab or if the low-vision/blind person should expect that clicking it means they leave this page in the same window. It's confusing as hell when it's the unmarked latter because they're like "wat" when the thing they thought they were looking at has unexpectedly changed.

Second - a recommendation for your social media is that you pick one layout for it and then copy/paste it in the footer everywhere you want it. There's inconsistencies across your pages right now - in your about page they're free-floating in a section outside the footer, they're in your footer with an id in your index page, and your projects page has no id on its footer.

Third - just a few lines up on 81 (under section class="project-foot") you misspelled a few things: "Link to Github repository."


header {
position: fixed;
background: #fff;

Choose a reason for hiding this comment

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

Clever! I didn't learn about abbreviating hex colors that had the same letter until much later.

Choose a reason for hiding this comment

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

Also, you should very rarely need to repeat yourself with regards to color in the same section (site header, for example). You don't always have to include the body for the header - you could just target the header itself to control its color. That way your code stays DRY and it's easier to read and make changes.


#logo {
float: left;
width: 100px;

Choose a reason for hiding this comment

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

Why did you use % width up above for header and pixel width for logo?


header ul {
float: right;
padding: 4em 2em 0 0;

Choose a reason for hiding this comment

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

I recommend using % for padding and borders and rem for font sizes - this is what Jamie strongly suggested.

margin: 0;
padding: 0;
display: inline;
height: 40px;

Choose a reason for hiding this comment

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

I'd recommend standardizing the sizing format (%, px, em/rem) you use to set up heights and widths. One thing I used to do when making my .css file was to take all of the classes and ids in a section and cross off duplicate lines - it helped me to see when things weren't as DRY as they could be. Having standardized sizing formats helped make it obvious when I was repeating myself unnecessarily.

You're missing H1 tags, which may cause screen readers to have a hard time orienting themselves on your site. Your .css file could use some trimming and DRYing up, but overall solid project. 👍

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.

3 participants