Remove flex-grid-column() duplicate max-width
#10341
Conversation
LeoColomb
changed the title
flex-grid-column() duplicate max-widthflex-grid-column() duplicate max-width
There was a problem hiding this comment.
Hi @LeoColomb. Thank you for your ✨ first contribution ✨ on Foundation !
I have some remarks to make in your PR. What do you think ?
If you agree on these change, will you be able to provide them more or less quickly ?
scss/grid/_flex-grid.scss
Outdated
| // https://stackoverflow.com/questions/34934586/white-space-nowrap-and-flexbox-did-not-work-in-chrome | ||
| @if $columns == expand { | ||
| min-width: 0; | ||
| } |
There was a problem hiding this comment.
Even if I understand the logic behind this move (all fixes in the same place), this Chrome fix should stay in the component mixin flex-grid-column, because it does not need to be re-generated when we apply a different size.
For example:
.my-flex-col {
@include flex-grid-column(12);
// ✓ The IE fix is generated for 12 columns.
// ✓ The Chrome fix is generated.
}
.my-flex-col--demo {
@include flex-grid-size(6);
// ✓ The IE fix must be generated for 6 columns.
// ☓ The Chrome fix still work, so is useless here.
}Please move back the Chrome fix to flex-grid-column.
There was a problem hiding this comment.
I see your point, fix moved back! 😉
Very small fix inside `flex-grid-column()` mixin: `max-width` fix is added twice: call to `flex-grid-size()` already import this fix once.
|
@kball Ready for merge I think. |
|
Hey @LeoColomb ... Congrats on your first merged PR 😉 Thanks for digging into it @ncoden 😉 |
Remove `flex-grid-column()` duplicate `max-width`
Very small fix inside
flex-grid-column()mixin:max-widthfix is added twice: call toflex-grid-size()already import this fix once.Also, the
min-widthfix should be insideflex-grid-size()for really fixing things for any size change.