-
Notifications
You must be signed in to change notification settings - Fork 70
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evanmwillhite This looks great! Thanks for including the css grid variation. I learned a lot just from playing with it.
As long as your okay with my changes, this is ready to merge!
@@ -35,8 +35,6 @@ $main-width: calc(100% - (#{$sidebar-width} + #{$gutter})); | |||
} | |||
|
|||
.main-content { | |||
width: 100%; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this back, because it is needed for the non-sidebar layout
// .main-sidebar { | ||
// grid-area: sidebar; | ||
// margin-bottom: $space-double; | ||
// margin-bottom: $gutter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like what's been done here, but I personally prefer to use space for bottom margins between things. I also like it to be space-double instead of just space visually. If you prefer to have these be the same, let's make a gutter-double that uses space-double. We could also just use space for all of these instead of making our own vars just for this file. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with this the way it is. This is just an example after-all. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the more vs less vertical space is probably a personal preference thing, so I'm not going to push back.
Let's !
I made some tweaks and added some comments. Let me know what you think. |
Addresses #244
Along with fourkitchens/emulsify-gulp#78, this PR removes singularity for layout and replaces it with a flexbox implementation with a CSS Grid one commented out.
What it does
To Test