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

Sea Turtles - Georgia A #94

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions about.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Georgia Adam</title>
<link rel="shortcut icon" href="/assets/forest.png">

Choose a reason for hiding this comment

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

Nice, I like that the favicon ties in with the header image 😄

<link rel="stylesheet" href="/styles/style.css">
<link rel="stylesheet" href="/styles/about.css">

</head>

<body>
<nav>
<ul>
<li><a href="/index.html"> Home </a></li>
<li><a href="/portfolio.html"> Portfolio </a></li>
<li><a href="/about.html"> About </a></li>
</ul>
</nav>
Comment on lines +16 to +22

Choose a reason for hiding this comment

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

Looking at the flow of the pages, I would consider the navigation as part of the header that starts off every page. Given that, I suggest moving these nav blocks into the header elements and styling as necessary so that the HTML better represents layout. This kind of organization can help folks with screen readers get a better understanding of the intended flow and content of a website.

Copy link
Author

Choose a reason for hiding this comment

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

Ok this makes sense! Good to know about the screen readers, I did not know that.


<header>
<h1>Georgia Adam</h1>
<h2>Software Engineering Student @ Ada Developers Academy</h2>
</header>

<section id="about_me">
<h3>
About me.
</h3>
Comment on lines +30 to +32

Choose a reason for hiding this comment

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

This is completely a matter of preference, but since this is a short piece of text, it would be okay to do inline like you have in the header above.

</section>

<main class="grid_container">

Choose a reason for hiding this comment

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

A main element is intended to represent the dominant content of the body, so I would consider including the section above inside of this main tag.

<div id="paw"><img alt="Pawprint" src="assets/track.png"/></div>
<div id="matcha"><img alt="Matcha" src="assets/matcha.png"/></div>
<h3 id="bio">Dog lover. Matcha drinker.</h3>
</main>

<section class="contact">
<h3>Contact</h3>
<ul>
<li><a href="mailto:[email protected]"><img alt="Email" src="assets/square.png"/></a></li>

Choose a reason for hiding this comment

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

Nice use of mailto for email without javascript!

<li><a href="https://www.linkedin.com/in/georgiaeadam/"><img alt="LinkedIn" src="assets/linkedin.png"/></a></li>
</ul>
</section>

<footer>
<h4>&copy; 2022 Georgia Adam <br>

Choose a reason for hiding this comment

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

It's considered better form to use css to create space between elements since <br> isn't very flexible. We want to be pretty sparing with our use of the br tag and save it for places where a line break is really meaningful (Examples from the MDN docs on br are poems and postal addresses).

A primary reason to avoid brs is that they can make navigation with screen readers more difficult. I recommend taking a look at the accessibility concerns section of the MDN's br reference. Their suggested replacement is to use individual <p> elements and use styling on properties like the margin to create space between them.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/br#accessibility_concerns

Photo by <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Amanda Frank</a> on <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Unsplash</a> /

Choose a reason for hiding this comment

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

Since this line is long and there's another set of anchor tags that you might need to scroll to see, I would recommend breaking this into 2 lines so it's harder to miss content when skimming down the HTML.

Suggested change
Photo by <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Amanda Frank</a> on <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Unsplash</a> /
Photo by <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Amanda Frank</a> /
on <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Unsplash</a> /

<a href="https://www.flaticon.com/free-icons/tree" title="tree icons">Tree icons created by Freepik - Flaticon</a> /
<a href="https://www.flaticon.com/free-icons/at" title="at icons">At icons created by Freepik - Flaticon</a> /
<a href="https://www.flaticon.com/free-icons/linkedin" title="linkedin icons">Linkedin icons created by Freepik - Flaticon</a> /
<a href="https://www.flaticon.com/free-icons/pet" title="pet icons">Pet icons created by Freepik - Flaticon</a> /
<a href="https://www.flaticon.com/free-icons/matcha" title="matcha icons">Matcha icons created by Vitaly Gorbachev - Flaticon</a>
</h4>
</footer>
</body>

</html>
Binary file added assets/amanda-frank-qLfyTluVZAY-unsplash.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/forest.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/linkedin.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/matcha.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/notepad-transparent-background-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/square.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/track.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
57 changes: 57 additions & 0 deletions index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Georgia Adam</title>
<link rel="shortcut icon" href="/assets/forest.png">
<link rel="stylesheet" href="/styles/style.css">
<link rel="stylesheet" href="/styles/index.css">

</head>

<body>
<nav>
<ul>
<li><a href="/index.html"> Home </a></li>
<li><a href="/portfolio.html"> Portfolio </a></li>
<li><a href="/about.html"> About </a></li>
</ul>
</nav>

<header>
<h1>Georgia Adam</h1>
<h2>Software Engineering Student @ Ada Developers Academy</h2>
</header>

<section id="welcome">
<h3>
I am student software engineer currently learning and growing at <a href="https://adadevelopersacademy.org">Ada Developers Academy</a>.
Presently, I am most interested in backend development and hardware programming. However, I am always open to learning more about
what can be created with code. Please reach out! I am open to new connections and would love to learn something new.<br>
<br>
Feel free to check out my work on the <a href="/portfolio.html">portfolio page</a>. I also talk about my interests outside of programming on the <a href="/about.html">about page</a>.
</h3>
</section>
Comment on lines +29 to +37

Choose a reason for hiding this comment

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

Since this is the primary content of the body, I would suggest using main over section here.


<section class="contact">
<h3>Contact</h3>
<ul>
<li><a href="mailto:[email protected]"><img alt="Email" src="assets/square.png"/></a></li>
<li><a href="https://www.linkedin.com/in/georgiaeadam/"><img alt="LinkedIn" src="assets/linkedin.png"/></a></li>
</ul>
</section>

<footer>
Comment on lines +39 to +47

Choose a reason for hiding this comment

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

Similar to the feedback about moving the nav into the header tag, I would recommend moving the contact section into the footer element below it, since the contact info seems to be a part of the conceptual footer on each page.

Copy link
Author

Choose a reason for hiding this comment

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

This makes sense. Thanks! Your comments are helping with my conceptualization of headers and footers in HTML and tying them more organically to what we see on the page.

<h4>&copy; 2022 Georgia Adam <br>
Photo by <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Amanda Frank</a> on <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Unsplash</a> /
<a href="https://www.flaticon.com/free-icons/tree" title="tree icons">Tree icons created by Freepik - Flaticon</a> /
<a href="https://www.flaticon.com/free-icons/at" title="at icons">At icons created by Freepik - Flaticon</a> /
<a href="https://www.flaticon.com/free-icons/linkedin" title="linkedin icons">Linkedin icons created by Freepik - Flaticon</a>
</h4>
</footer>
</body>

</html>
12 changes: 0 additions & 12 deletions pages/about.html

This file was deleted.

12 changes: 0 additions & 12 deletions pages/index.html

This file was deleted.

12 changes: 0 additions & 12 deletions pages/portfolio.html

This file was deleted.

60 changes: 60 additions & 0 deletions portfolio.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Georgia Adam</title>
<link rel="shortcut icon" href="/assets/forest.png">
<link rel="stylesheet" href="/styles/style.css">
<link rel="stylesheet" href="/styles/portfolio.css">

</head>

<body>
<nav>
<ul>
<li><a href="/index.html"> Home </a></li>
<li><a href="/portfolio.html"> Portfolio </a></li>
<li><a href="/about.html"> About </a></li>
</ul>
</nav>

<header>
<h1>Georgia Adam</h1>
<h2>Software Engineering Student @ Ada Developers Academy</h2>
</header>

<section id="blurb">
<h3>
Please find a selection of projects I have worked on below.
</h3>
</section>
Comment on lines +29 to +33

Choose a reason for hiding this comment

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

I would suggest including this section inside the main element below since it is the header for the dominant content of the body.


<main class="flex_container">
<div><a href="https://github.com/georgia-adam/task-list-api"><h2>Task List</h2></a><h3>Flask API</h3></div>
<div><a href="https://github.com/georgia-adam/book-review-site"><h2>Book Review Site</h2></a><h3>HTML/CSS</h3></div>
<div><a href="https://github.com/georgia-adam/swap-meet"><h2>Swap Meet</h2></a><h3>Python OOP</h3></div>
<div><a href="https://github.com/georgia-adam/viewing-party"><h2>Viewing Party</h2></a><h3>Python</h3></div>
Comment on lines +36 to +39

Choose a reason for hiding this comment

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

  • We should avoid div unless it is purely for styling. Since each of these divs seems to have meaning through grouping together a block of content that represents a project, I would strongly suggest replacing the div tags with something like article or section.

  • This is conceptually a list of projects, so I would consider wrapping them in a list element and styling as needed to achieve the desired layout.

  • There are several tags on the same line, I think it would help the structure be clearer when reading the HTML if we broke these down into multiple lines:

    <div>
        <a href="https://github.com/georgia-adam/task-list-api">
            <h2>Task List</h2>
        </a>
        <h3>Flask API</h3>
    </div>

</main>

<section class="contact">
<h3>Contact</h3>
<ul>
<li><a href="mailto:[email protected]"><img alt="Email" src="assets/square.png"/></a></li>
<li><a href="https://www.linkedin.com/in/georgiaeadam/"><img alt="LinkedIn" src="assets/linkedin.png"/></a></li>
Comment on lines +45 to +46

Choose a reason for hiding this comment

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

These lines are pretty long, I would consider breaking them up across multiple lines (which has the added benefit of making the nested structures clearer).

</ul>
</section>

<footer>
<h4>&copy; 2022 Georgia Adam <br>
Photo by <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Amanda Frank</a> on <a href="https://unsplash.com/@aewild?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Unsplash</a> /
<a href="https://www.flaticon.com/free-icons/tree" title="tree icons">Tree icons created by Freepik - Flaticon</a> /
<a href="https://www.flaticon.com/free-icons/at" title="at icons">At icons created by Freepik - Flaticon</a> /
<a href="https://www.flaticon.com/free-icons/linkedin" title="linkedin icons">Linkedin icons created by Freepik - Flaticon</a>
</h4>
Comment on lines +51 to +56

Choose a reason for hiding this comment

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

The h1-h6 tags are intended to represent the heading of a section. Here the main content of the footer is embedded in the h4 element. This can make it harder for screen reader users to understand and navigate the content. I would recommend using one or more

elements here and styling as needed to get the boldness and such you want from the font.

</footer>
</body>

</html>
36 changes: 36 additions & 0 deletions styles/about.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
.grid_container {
width: auto;
height: auto;
display: grid;
grid-template: 0.25fr 1fr / 1fr 3fr;
margin-right: 30%;
}

.grid_container > div {
padding: 50px;
}

#bio {
grid-column: 2;
grid-row: 1 / 3;
padding-left: 30px;
padding-top: 12%;
font-size: x-large;
background: url(/assets/notepad-transparent-background-1.png);
background-repeat: no-repeat;
background-position: top;
background-size: cover;
border-left: 4px solid black;
font-family: cursive;
}
Comment on lines +13 to +25

Choose a reason for hiding this comment

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

This turned out really well, nice use of styling ^_^


#about_me {
margin-left: 2px;
margin-right: 30%;
padding-bottom: 10px;
font-size: large;
}

#paw {
border-bottom: 4px solid black;
}
5 changes: 5 additions & 0 deletions styles/index.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#welcome {
font-size: large;
margin-left: 2px;
margin-right: 30%;
}
32 changes: 32 additions & 0 deletions styles/portfolio.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
.flex_container {
display: flex;
flex-direction: row;
flex-wrap: wrap;
margin-right: 30%;
justify-content: space-between;
align-items: center;
border: 4px solid black;
background-color: grey;
}

.flex_container > div {
border: 4px solid black;
width: 150px;
height: 150px;
margin: 2px;
flex: 1 1 auto;
padding: 50px;
text-align: center;
background-color: whitesmoke;
}

.flex_container h3 {
text-decoration: overline;
}

#blurb {
margin-left: 2px;
margin-right: 30%;
padding-bottom: 10px;
font-size: large;
}
49 changes: 49 additions & 0 deletions styles/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
* {

Choose a reason for hiding this comment

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

Nice organization for the styles to reduce repetition!

font-family: monospace;
}

ul {
list-style-type: none;
margin: 0;
padding: 0;
}

ul > li {
display: inline-block;
}

nav {
padding-top: 10px;
padding-bottom: 15px;
margin-right: 30%;
margin-left: 2px;
}

header {
background: url(/assets/amanda-frank-qLfyTluVZAY-unsplash.jpg);
background-repeat: no-repeat;
background-size: cover;
background-position: center;
border: 4px solid black;
color: #ffffff;
text-shadow: -1px 0 black, 0 1px black, 1px 0 black, 0 -1px black;
padding-top: 40px;
padding-bottom: 40px;
padding-left: 10px;
margin-right: 30%;
}

.contact {
margin-left: 2px;
margin-right: 30%;
padding-top: 20px;
}

.contact > h3 {
text-decoration: underline;
}

footer {
margin-left: 2px;
margin-right: 30%;
}