Skip to content

Kimberly Project 2#32

Open
Kiimby3 wants to merge 1 commit into
Technigo:masterfrom
Kiimby3:master
Open

Kimberly Project 2#32
Kiimby3 wants to merge 1 commit into
Technigo:masterfrom
Kiimby3:master

Conversation

@Kiimby3
Copy link
Copy Markdown

@Kiimby3 Kiimby3 commented Nov 5, 2023

No description provided.

Copy link
Copy Markdown

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

Good job with your page! Maybe you want to comment back in the styling for the big about-image? Keep up the good work! /Matilda

Comment thread code/index.html
Comment on lines +11 to +15
<ul class="navigation-list">
<li><a href="articles.html">Articles</a></li>
<li><a href="about.asp">About</a></li>
<li><a href="contact.asp">Contact</a></li>
</ul>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of ul, you could use the nav element here

Comment thread code/index.html
<main class="">
<!-- The main container to use as the flex parent for your card layout, the main content of the site should be in here. -->
</main>
<img src="images/comics.jpg" alt="Comic books" class="top-image" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good that you're using alt tags!

Comment thread code/index.html

<div class="about-container">
<h1>About</h1>
<img src="images/shop.jpg" alt="Shop" class="about-image" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This image is veeery big, maybe style it smaller? It's causing a major side scroll to your page 😅

Comment thread code/style.css
Comment on lines +87 to +94
/*
.about-image {
max-width: 50%;
max-height: 300px;
float: left;
object-fit: cover;
}
*/
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Aah, I see you commented this out 👀

Comment thread code/style.css
Comment on lines +8 to +18
.navigation-list {
float: right;
list-style-type: none;
margin: 0;
padding: 0;
overflow: auto;
background-color: #f0bbcd;
position: fixed;
top: 0;
width: 100%;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice to have this fixed 👍

Comment thread code/style.css
Comment on lines +56 to +67
.article-container {
max-width: 66%;
margin: auto;
display: flex;
align-items: center;
align-content: center;
flex-direction: row;
flex-wrap: wrap;
justify-content: space-between;
padding: 40px;
background-color: #f0bbcd;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice usage of flexbox ⭐️

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