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

tigers L.MORA personal site #122

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

Conversation

lilianashley
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?
Why is it important to consider and use semantic HTML?
How did you decide to structure your CSS?
What was the most challenging piece of this assignment?
Describe one area that you gained more clarity on when completing this assignment
Optional
Did you deploy to GitHub Pages? If so, what is the URL to your website?

Copy link

@spitsfire spitsfire left a comment

Choose a reason for hiding this comment

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

Nice, Lily! There are a few things you should consider for future front-end development:

  • Consider using a HTML validator to help you find mistakes and missed tags in your HTML.
  • Make sure your element attributes do not have space around their = assignment.

Comment on lines +10 to +13
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Shadows+Into+Light&display=swap" rel="stylesheet">
</head>
<body>

<body class="color">

Choose a reason for hiding this comment

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

These should be indented over one more time


</ul>
</nav>
</header>

Choose a reason for hiding this comment

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

You've got a closing header and nav tag, but not an opening one. It's a good idea to run your HTML through a validator to catch these mistakes.

</nav>
</header>
<div class="grid-container">
<div>

Choose a reason for hiding this comment

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

Let's use section rather than div as much as we can

<p></p>
</div>
<div><img class="photos" src="img/pl.jpg">
<p><MI AMOR><p></div>

Choose a reason for hiding this comment

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

What's happening here? p needs a closing tag.

<div><img class="photos" src="img/pl.jpg">
<p><MI AMOR><p></div>
</div>
<header>

Choose a reason for hiding this comment

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

This should be above its matching closing tag

</li>
</ul>
</nav>
<center><h1>LILIAN ASHLEY MORA</h1></center>

Choose a reason for hiding this comment

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

center is going out of style and should be replaced with the css styling rule text-align: center

Comment on lines +30 to +33
<div>FOODIE</div>

<div>TRAVEL</div>
<div>MUSIC</div>

Choose a reason for hiding this comment

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

div is used to divide elements, not display text. So let's put FOODIE and such inside a heading or paragraph tag inside the div. Or even better, replace div with section

<img class="photos" src="img/github.gif">
</div>
<div class="flex-container2">
<Ol>

Choose a reason for hiding this comment

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

tags are always lowercase: <ol>

Comment on lines +6 to +9
.grid-container {
display: grid;
grid-template-columns: auto auto auto;
}

Choose a reason for hiding this comment

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

Good job using css grid! 👍

Comment on lines +29 to +39
.flex-container {

display: flex;
justify-content: space-between;

}
.flex-container2 {

display: flex;
justify-content: center
}

Choose a reason for hiding this comment

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

Glad you used some flexbox styling! 👍

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