search

incuna / incuna-sass

Incuna's Sass Library
MIT License
2 stars 2 forks source link

Add grid mixin for creating grid backgrounds with css gradients #80

Closed hippogriffic closed 8 years ago

hippogriffic commented 8 years ago

@incuna/frontend please review

LibbyChapman commented 8 years ago

👍 This looks clever and fun, nice one @hippogriffic 🎈

perry commented 8 years ago

I think that the mixin should be renamed, as I immediately thought this was a mixin for grid layouts/structures.

pandalion commented 8 years ago

Ahh, @perry has a good point (dammit)

I think it might be nice and make sense to have a "background patterns" mixins file, where we can put useful things like this, but then maybe have various ones in it (or I guess it could be a folder, maybe mixins - bg-patterns, or something. Not sure what you could rename it to though.. if it was a sass file rather than a folder (eg mixins > bg-patterns.sass) you could just comment that this one is a grid, and the actual mixin could be something like @include bg-pattern-grid I think that would avoid confusion with grids in the generally used sense of the word.

hippogriffic commented 8 years ago

hmm yeah I didn't think of that, definitely renaming it to specify it's a background seems reasonable

would bg-grid suffice? putting pattern in there seems really wordy to me but idk, thoughts?

and yeah @pandalion I like the idea of having more mixins like this, maybe I can just rename the file to bg-patterns or bg-mixins for now and if we get loads then we can make it a folder instead?

pandalion commented 8 years ago

I would go with renaming the file bg-patterns and rename the mixin bg-grid inside that, is that okay? :) I mean personally I still prefer bg-grid-pattern because I think clarity of what it does is more important than how long it is, but.. I don't mind too much. I'm just thinking if you see it in another css file down the line then seeing @include bg-grid-pattern is clearer to what it does for somebody with no prior knowledge where as seeing @include bg-grid could look like "oh it's doing some actual css grid cleverness"

hippogriffic commented 8 years ago

yeah ok you've won me over

hippogriffic commented 8 years ago

renamed all the things @incuna/frontend @perry :D

pandalion commented 8 years ago

nice :D

pandalion commented 8 years ago

@hippogriffic minor gripe, could we maybe change the changelog too "Add mixin for creating grid pattern backgrounds with css gradients"