Skip to content

Branches - Kelsey#34

Open
kelsk wants to merge 6 commits into
Ada-C12:masterfrom
kelsk:master
Open

Branches - Kelsey#34
kelsk wants to merge 6 commits into
Ada-C12:masterfrom
kelsk:master

Conversation

@kelsk

@kelsk kelsk commented Sep 20, 2019

Copy link
Copy Markdown

Startrly

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
Describe a time you chose to use flexbox I used flexbox in the Sponsors section, pretty normally.
Describe a time you chose to use CSS Grid I initially used flexbox for the Team section, then switched to Grid because I prefer Grid.
What was a challenge you overcame in this project? A challenge I overcame in this project was getting the proper layout on the Why? section. I used Grid to create rows and columns and I manipulated the span and sizes to get everything to fit.
What concept did you get the most clarity on through Startrly? I got much more clarity on flexbox but it is still not my favorite thing in the world.

@krismosk krismosk left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Amazing job!

Comment thread index.html


<section>
<section class="quote">

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 job on re-using classes for these blockquotes! This achieves DRY'er code.

Comment thread styles-mobile.css
@@ -0,0 +1,54 @@
@media only screen and (max-width: 630px) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like that you implemented something new that wasn't specifically mentioned in the project requirements (but could be inferred since they provided us with a wireframe for desktop and mobile). Super clever!

Comment thread styles.css
margin: 61px;
}

/* QUOTES */

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 job on adding comments for signposting. This is helpful for my review.

Comment thread styles-mobile.css
.tellmewhy img {
display: none;
}
.tellmewhy ul li {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would consider adding some comments for sign-posting, like you did in your styles.css. This would help me understand what elements are being changed from your website without having to reference the html.

Comment thread styles.css
padding: 0 5em 0 5em;
text-align: right;
}
nav ul li:first-child {

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 job using accessors.

Comment thread styles.css
border-bottom: none;
}
.team ul {
display: grid;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Super cool you made this into a grid. I had trouble doing this myself, and chose a flexbox. It's cool you got this to work!

Comment thread styles.css
align-items: center;
}

/* SUPPORT */

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We already chatted about this, but I would consider changing the name of your class to make it more clear that it is formatting both the support and about section. Otherwise, good job!

Comment thread styles-mobile.css
grid-template: 1fr / 1vw 55vw 15vw 15vw;
}


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 is minor (your code is so good that I'm looking for things to critique).. but I would consider cleaning up your extra spaces to make the file nice and clean to read. I think single spaces might be enough to make it readable.

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