Skip to content

Leaves - Yitgop#44

Open
rinostar wants to merge 4 commits intoAda-C12:masterfrom
rinostar:master
Open

Leaves - Yitgop#44
rinostar wants to merge 4 commits intoAda-C12:masterfrom
rinostar:master

Conversation

@rinostar
Copy link

@rinostar rinostar commented Sep 20, 2019

Startrly

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
Describe a time you chose to use flexbox I used flexbox for the overall structure and each sections
Describe a time you chose to use CSS Grid N/A
What was a challenge you overcame in this project? Understand and apply the differences between flexbox and CSS Grid
What concept did you get the most clarity on through Startrly? Flexbox

Copy link

@sun-alice sun-alice left a comment

Choose a reason for hiding this comment

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

Very nice job!! It looks very similar to the site we were supposed to recreate, excellent color scheme. I like your use of flex displays, it worked really well in your title/meet the team/sponsor sections. The sections could maybe be named something else other than section_1, 2, 3-- it was a little challenging identifying which css designs were for what section.



<section>
<section class="section_4">

Choose a reason for hiding this comment

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

section_2 and section_4 are both quotes, and are formatted similarly, so maybe they could be a class!

<nav>
<div class="container">

<nav class="nav">

Choose a reason for hiding this comment

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

i think because the class="nav" is within class="container", you only need to have one class here.
e.g. if you had just div class="container", you can access the "nav" by:

.container nav {
   #formatting stuff
}

text-align: center;
}

.section_5 ul {

Choose a reason for hiding this comment

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

very nice use of flex!

}

.section_5 img {
border-radius: 50%;

Choose a reason for hiding this comment

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

i enjoy a circular bear photo

list-style: none;
display: flex;
justify-content: row;
flex-wrap: wrap;

Choose a reason for hiding this comment

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

very nice sponsor section, it expands when I expand my browser too

}

.section_3 {
display: flex;

Choose a reason for hiding this comment

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

I had a hard time getting the last list element ("convenient") to stay on the right side of the image too. i ended up using a grid!

.section_2 {
padding: 40px;
border-top: 3px solid lightgray;
border-bottom: 3px solid lightgray;

Choose a reason for hiding this comment

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

there is a border on many of your sections, so you could maybe add a general border to all sections to dry up the code a little

e.g.

section {
  border-bottom: 2px solid lightgrey; 
}

}

.section_7 ul {
list-style: none;

Choose a reason for hiding this comment

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

you have list-style: none; in a few sections that have ul's, this could also maybe be moved into a general ul attribute to dry up code a little!

e.g.

ul {
  list-style-type: none;
}

}

.nav {
background-color: lightgray;

Choose a reason for hiding this comment

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

good choice of color

}

.nav ul li:hover {
background-color: lightsalmon;

Choose a reason for hiding this comment

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

nice touch!

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