Merge omega and nth-omega with span-columns #32

Closed
kaishin opened this Issue Nov 7, 2012 · 5 comments

2 participants

@kaishin
thoughtbot, inc. member

This would make call omega() or nth-omega() inside span-columns(). This would be something like:

@include span-columns(6 of 8, omega);
@icnlude span-columns(2, 4n);
@dukex

Nice! I like it!

I prefer omega inline

div {
   @include span-columns(6 of 8);
   + div{
       @include omega;
   }
}

Instead of span-columns with omega

div {
   @include span-columns(6 of 8);
   + div{
       @include span-columns(6 of 8, omega);
   }
}

do you see a case to use span-columns with omega?

@kaishin
thoughtbot, inc. member

Even if they are merged, the omega() mixin will remain intact, so you'd still be able to use it inline. I guess my concern at this point is backward compatibility more than anything else: span-columns(columns, display) already takes two arguments, so adding a third won't happen anytime before 2.x.

The intermediate step could be refactoring omega() and nth-omega() into one single mixin.

@dukex

Makes sense. Change span-columns now will break compatibility, I think on 2.x the display parameter will be depreciated with new outer-container.

Trying think that refactoring: I think should be depreciate nth-omega and use only omega mixing, maybe omega should receive more one argument(nth), like it nth-omega:

@include omega(4n)

the mixin:

@mixin omega($nth, $display: block, $direction: right){
....
}

the problem is backward compatibility, but for now one or two @if should resolve

@dukex

I tried make omega and nth-omega merge ;)

@kaishin
thoughtbot, inc. member

Mixins omega and nth-omega were merged in bb084c5, but were not merged with span-columns to avoid overloading the latter.
Closing this for now.

@kaishin kaishin closed this Feb 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment