Skip to content

Conversation

@jggarza5
Copy link

@jggarza5 jggarza5 commented Apr 4, 2018

Challenge #2

Copy link

@enitchals enitchals left a comment

Choose a reason for hiding this comment

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

Nice work with this assignment!

time, mark, audio, video {
margin: 0;
padding: 0;
margin: 20;

Choose a reason for hiding this comment

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

Adding properties to a bunch of elements/classes at once is really efficient and it's a good coding practice when there's a use for it,, but I do want to caution you to think carefully about inheritance when you assign them like this. Many properties are inherited, but some are not -- and if the code you're writing applies to nearly every element on the page, it can be hard to predict and troubleshoot the changes you're making.

}

.container {
width: 880px;

Choose a reason for hiding this comment

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

Adding a width to the container is a great way to make a predictable layout that will look the same every time. As we move into responsive layouts, I encourage you to think about the pros and cons of a fixed-width content container versus a responsive layout (or even just a different CSS property, like adding margins to the container).

display: flex;
flex-direction: row;
justify-content: flex-start;
justify-content: space-evenly;

Choose a reason for hiding this comment

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

It looks like you're assigning the justify-content property and then overwriting it on the next line. This shouldn't cause any problems while rendering, but it might be confusing to look at later on.

@mixelpixel mixelpixel closed this May 7, 2018
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.

3 participants