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

Sharks - Camilla #104

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

Conversation

camilla122333
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? Yes, fixed. The most interesting one was footer (flex-item) was outside body (flex-container) in 1 page.
Why is it important to consider and use semantic HTML? To make it easier for ourselves and others who review/look at our code.
How did you decide to structure your CSS? 2 different pages. I tried to maintain alphabetical order for elements, then add classes and extras after.
What was the most challenging piece of this assignment? Having a compelling vision for the main page. Once I had that it was a bit frustrating, but there was forward momentum and progress, and I knew what to do. I had to restart from scratch, as my first attempt was so uncompelling that I did not know what to do with it!
Describe one area that you gained more clarity on when completing this assignment Flexbox. Also, wireframing.
Optional
Did you deploy to GitHub Pages? If so, what is the URL to your website? No.

Copy link

@yangashley yangashley left a 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! Glad you got some practice in with Grid and Flexbox.

Nice job avoiding non semantic tags like div and span for styling. I made a few suggestions about more semantically relevant elements that you could use in a few places.

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>
<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>Document</title>
<link rel="stylesheet" type="text/css" href="../styles/style-flexbox.css">
<title>Camilla Berretta Dev</title>

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.

Comment on lines +22 to +24
<h1 class="othertitle">
ABOUT
</h1>

Choose a reason for hiding this comment

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

It depends on who you ask, but your document should have only one h1 element. Of course you can add multiple and the browser will still render the h1 tags, but it is more semantically correct to only use 1 since heading tags have a hierarchy

Since you already have an h1 element on lines 15-17, you can use a h2 here.

Also, it's perfectly fine to have opening tag, content, and closing tag all on one line if it's not too long, like:

<h1 class="othertitle">ABOUT</h1>

pages/index.html Outdated
Comment on lines 22 to 27
<p>
<a href="../pages/portfolio3.html">
<img src="../assets/cheetah.png" alt="cheetah" class="center greyscalehover">
portfolio
</a>
</p>

Choose a reason for hiding this comment

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

I think a section tag would make more sense semantically since a p tag represents a paragraph

Comment on lines 19 to 20
<section>
</section>

Choose a reason for hiding this comment

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

Looks like these are unused section tags that can be deleted

</section>

<article>
<h1>My Portfolio</h1>

Choose a reason for hiding this comment

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

You can use a different header tag here since you've already got a h1 element on the page.

</p>
</nav>

</article>

Choose a reason for hiding this comment

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

I think that the closing article tag should come directly after your block of text ends on line 44. Since an article element usually specifies independent, self-contained content.

Then you'd have your nav element with your projects as a sibling of article, instead of nested in article

Comment on lines 46 to 65
<nav class="box flex">
<p>
<a href="https://github.com/camilla122333/task-list-api">
<img src="../assets/annie-sprattpinkbackground.jpg" alt="project" class="center">
tasklist
</a>
</p>
<p>
<a href="https://github.com/camilla122333/viewing-party">
<img src="../assets/jason-leung-wmyE5IBiOmo-unsplash.jpg" alt="project" class="center">
viewing party
</a>
</p>
<p>
<a href="https://www.pasadenastringtheory.com/recordings">
<img src="../assets/annie-sprattpinkbackground.jpg" alt="project" class="center">
music projects
</a>
</p>
</nav>

Choose a reason for hiding this comment

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

I think using an unordered list to encapsulate your projects or even a section element (instead of a nav element) may work better here.

A nav element represents a section of a page whose purpose is to provide navigation links, either within the current document or to other documents. So someone who is using assistive technology, might navigate to this part of the site and expect to be able to navigate to other parts of your site (home, about pages, etc) but then they would be redirected to github instead.

If you used an unordered list, then each list item could be a section, like:

<ul>
  <li>
    <section>
          <a href="https://github.com/camilla122333/task-list-api">
            <img src="../assets/annie-sprattpinkbackground.jpg" alt="project" class="center">
            tasklist
          </a>
    </section>
  </li>
</ul>

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