-
Notifications
You must be signed in to change notification settings - Fork 110
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
Sharks - Ivana #89
base: master
Are you sure you want to change the base?
Sharks - Ivana #89
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your site looks very nice! Nice job practicing Grid and Flexbox!
I left suggestions about how you can structure your HTML content to make it more semantic (avoid using divs and spans when you can, using more meaningful elements like header/footer, and restructuring some elements so they're nested more accurately).
Whenever you have the chance to revisit frontend development here's a great list of different resources to check out:
https://dev.to/nickytonline/frontend-developer-resources-2022-4cp2
🟢 for personal portfolio!
<head> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index.html page generally lives at the top level of the project directory (not within a subdirectory). When we visit a website (e.g., www.github.com) when no page is part of the URL, the web server will look for one of a few default file names (index.html is usually among them) and serve the first one it finds.
While a web server can be configured to serve a default file from elsewhere (such as under the pages folder) it's often easier to have the index.html live in the site root, and have the other pages stored under the pages folder. This affects the paths required for locating style files and images, or other pages in anchor tags.
<img src="/images/Profile.png" alt="Profile Picture" id="profile-pic" /> | ||
<h2>About Me</h2> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to avoid using non semantic elements like div or span unless absolutely necessary. I think you could get rid of this div here and put tags on these elements if you need to select them for styling. If you do need a parent element around the image and h2 tags on lines 24 and 25, can you think of a more semantically relevant one?
<section class="about-me-content"> | ||
<h5>I am a Software Development student based in Boston.</h5> | ||
<div class="about-paragraph"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could replace this div with another section element for more semantic meaning
<footer> | ||
<ul> | ||
<li></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can delete this unused list item
|
||
<footer> | ||
<ul> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can surround this unordered list with a nav element too and in the future you can add more links to it like your socials or linkedin/github
|
||
<body> | ||
<div class="container"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are you using this div for? Do you need it?
Consider removing it or replacing it with something more semantic
<h3>Software Development Student</h3> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove this div and put the "my-title" class on the h3?
<p class="home-page-paragraph"> | ||
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do | ||
eiusmod tempor incididunt ut labore et dolore magna aliqua. Quis | ||
ipsum suspendisse ultrices gravida dictum fusce. | ||
</p> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the div here and put "home-page-paragraph" on the p tag?
You could replace this div element with a section element
<div> | ||
<a href="https://github.com/maldonado-ivana/adagrams-py" | ||
><img src="/images/adagrams.png" alt="Adagrams project" | ||
/></a> | ||
<p>Adagrams</p> | ||
</div> | ||
<div> | ||
<a href="https://github.com/maldonado-ivana/viewing-party" | ||
><img src="/images/viewing_party.png" alt="Viewing Party project" | ||
/></a> | ||
<p>Viewing Party</p> | ||
</div> | ||
<div> | ||
<a href="https://github.com/maldonado-ivana/swap-meet" | ||
><img src="/images/swap-meet.png" alt="Swap meet project" | ||
/></a> | ||
<p>Swap Meet</p> | ||
</div> | ||
<div> | ||
<a href="https://github.com/maldonado-ivana/task-list-api" | ||
><img src="/images/task_list.png" alt="Task List project" | ||
/></a> | ||
<p>Task List</p> | ||
</div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using an unordered list here instead of a div for a grid-container.
Then you can have each project be a list item and then the a, p, image tag can be in the section (and they can be siblings too)
<ul>
<li>
<section>
<a href="https://github.com/maldonado-ivana/adagrams-py">
<img src="/images/adagrams.png" alt="Adagrams project"/>
<p>Adagrams</p>
</section>
</li>
<li>
<section>
<a href="https://github.com/maldonado-ivana/viewing-party">
<img src="/images/viewing_party.png" alt="Viewing Party project"/>
<p>Viewing Party</p>
</section>
</li>
</ul>
Personal Portfolio Site
Congratulations! You're submitting your assignment!
Comprehension Questions