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

Accelerate- Christina S. #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Peacegypsy
Copy link

No description provided.

Comment on lines +15 to +20
<nav>
<a href="../index.html"> Home</a>
<a href="../about.html">About</a>
<a href="../portfolio.html"> Portfolio</a>
<a href="../recipes.html"> Recipes</a>
<a href="../blog.html"> Words</a>
Copy link

@tgoslee tgoslee Jul 5, 2021

Choose a reason for hiding this comment

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

You can use jquery to make your nav bar reusable on each html page. Here is an example your nav info would be stored in nav.html and then a few lines of code would be added to each html file

Comment on lines +46 to +51
<a href="#" class="fa fa-facebook"></a>
<a href="#" class="fa fa-twitter"></a>
<a href="#" class="fa fa-google"></a>
<a href="#" class="fa fa-linkedin"></a>
<a href="#" class="fa fa-instagram"></a>
<a href="#" class="fa fa-pinterest"></a>
Copy link

Choose a reason for hiding this comment

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

i love font awesome icons!

Comment on lines +55 to +61
<a href="#" class="fa fa-facebook"></a>
<a href="#" class="fa fa-twitter"></a>
<a href="#" class="fa fa-google"></a>
<a href="#" class="fa fa-linkedin"></a>
<a href="#" class="fa fa-instagram"></a>
<a href="#" class="fa fa-pinterest"></a>
</p>
Copy link

Choose a reason for hiding this comment

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

Look into the jquery resource I shared with you above. You can also make your footer reusable.

Comment on lines +24 to +29
<h4>Nail Art</h4>
<img src="images/nail-art-1.png" alt="black nail polish with floral detail">
<img src="images/nail-art-2.png" alt="light neutral nail polish with gold flecks">
<img src="images/nail-art-3.png" alt="black and white nail polish">
<img src="images/nail-art-4.png" alt="white nail polish with multicolor detailing">
<img src="images/nail-art-5.png" alt="white nail polsih with blue flower petals">
Copy link

Choose a reason for hiding this comment

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

great use of alt! Best practice for img elements.

Comment on lines +30 to +39
<ul>
<li> 1/2 cup Almond butter, creamy</li>
<li> 1/3 tbsp Maple syrup, pure</li>
<li>1/4 cup Cocoa powder</li>
<li>2/3 cup Coconut flour</li>
<li>1/4 tsp Kosher salt</li>
<li>1/4 tsp Sea salt, flaky</li>
<li>1 tsp Vanilla extract</li>
<li>1 cup Coconut oil</li>
</ul>
Copy link

Choose a reason for hiding this comment

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

great use of list tag

</article><br>
<br>

<article class="article-2">
Copy link

Choose a reason for hiding this comment

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

article tag is great for big chunks of text. It could include h1 - h6 tags as its children. Great implementation.

background: #ddd;
margin-top: 20px;
}
.fa {
Copy link

Choose a reason for hiding this comment

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

since all of the colors are the same you can add color: white; here

Comment on lines +1 to +5
/* font-family: 'Aref Ruqaa', serif;
font-family: 'Italiana', serif;
font-family: 'Josefin Slab', serif;
font-family: 'New Tegomin', serif;
font-family: 'ZCOOL XiaoWei', serif;
Copy link

Choose a reason for hiding this comment

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

remove unused code

Comment on lines +44 to +59
/* portfolio page */

#img-box-1 {
display: flex;
flex-wrap: wrap;
justify-content:center;
margin: 20px;
}

#img-box-2 {
display: flex;
flex-wrap: wrap;
flex-direction: row;
justify-content: space-around;
margin-left: 25px;
}
Copy link

Choose a reason for hiding this comment

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

I like the layout of your about page and I suggest modeling your portfolio page in a similar way. I would also use different images/just the images once instead of repeating them.

nav {
position: sticky;
top: 0;
z-index: +1;
Copy link

Choose a reason for hiding this comment

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

z-index is just an integer no need for +

@tgoslee
Copy link

tgoslee commented Jul 5, 2021

Overall Great Job! I liked learning more about you and the layout of your about page in particular. I just made a few comments on how to make your header/footer reusable and cleaning some parts of the code :)

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