Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confusing gutter-span parameters order #183

Open
pepelsbey opened this issue May 6, 2014 · 12 comments
Open

Confusing gutter-span parameters order #183

pepelsbey opened this issue May 6, 2014 · 12 comments

Comments

@pepelsbey
Copy link

There are two $grid and $gutter parameters available for grid helpers. They act the same but for some reason gutter-span helper have a different order of parameters comparing to column-span and grid-span.

column-span($span, $location, $grid, $gutter)
grid-span($span, $location, $grid, $gutter)
gutter-span($gutter, $grid)

You can do something like this column-span(1, 2, 5) or grid-span(1, 2, 5) without mentioning $gutter, but when you just need to pass different $grid to gutter-span you have to go like this gutter-span(false, 5) instead of just gutter-span(5).

So there are two problems:

  1. It’s confusing because of inconsistency, similar order for similar parameters is usually expected.
  2. It makes you write more, in most cases you just need to change $grid but leave $gutter the same.

Is there any reason for that? If not I would really like you to change it.

@lolmaus
Copy link
Contributor

lolmaus commented May 6, 2014

Those arguments are optional. When they are omitted, Singularity takes the values from the grid definition above.

I don't mean that i support an inconsistency, but here are two ways to consider:

  1. Sass supports named arguments, so you can do gutter-span($gutter: .3).
  2. If you're overriding arguments twice or more times, wrap your code with the layout mixin and override in layout's arguments.

@pepelsbey
Copy link
Author

I tried to change order manually in following files:

  • api/_float.scss, :31
  • api/_isolation.scss :24
  • helpers/_background-grid.scss :176
  • math/_gutters.scss :1
  • math/_grid.scss :2

I it seems to work now: gutter-span(5) is not failing anymore without false.

@pepelsbey
Copy link
Author

@lolmaus, thank you for your suggestions, I haven’t seen named arguments in such context before. But the point of this issue is not to make my code just work (it’s fine already), but to improve API, documentation and code readability for the rest of users.

@lolmaus
Copy link
Contributor

lolmaus commented May 6, 2014

If you already did the code changes, consider filing a pull request and see whether @Snugug will accept it.

@pepelsbey
Copy link
Author

When they are omitted, Singularity takes the values from the grid definition above.

Wait a second. For the following nesting fragment:

<div class="foo">
    <div class="bar">

width value calculated with gutter-span() will be different if I omit values

@include add-grid(8);
@include add-gutter(1/5);

.foo {
    @include grid-span(5, 2);
    width:gutter-span();
    }
    .bar {
        width:gutter-span();
        }

But following code is restoring context

.bar {
    width:gutter-span(false, 5);
    }

So I want it to be not gutter-span(false, 5) but gutter-span(5) as restoring context operation is needed quite often.

@pepelsbey
Copy link
Author

If you already did the code changes, consider filing a pull request

I prefer to discuss it first, but sure, will do.

@lolmaus
Copy link
Contributor

lolmaus commented May 6, 2014

Can you please provide a http://SassMeister.com demo of you're trying to achieve with this approach?

@pepelsbey
Copy link
Author

@lolmaus, there you go.

I’m trying to get gutter width for margin-left using gutter-span(). When I specify no arguments it gives me width:X% based for global grid context, but inside block-one it’s calculated based on parent’s width, not global. So I have to specify context by passing $grid value, but I also have to specify $gutter ratio first (without any reason), see block-two and gutter-span(false, 2).

@lolmaus
Copy link
Contributor

lolmaus commented May 7, 2014

Ok, now i see that providing $grid for gutter-span() is indeed a more frequent scenario. I can't imagine a situation where i would need a gutter-span with an overridden gutter ratio only.

But i kinda don't understand your use case. Y r u doing dis? Can you show a pencil sketch of a layout where this technique makes sense?

I mean, you don't just provide an arbitrary offset to you inner elements, you want it to be consistent with your grid. But this makes elements lay off the grid and i don't understand how this is better than arbitrary margin-left: 20%.

@pepelsbey
Copy link
Author

I don’t think it’s worth to explain all my layout difficulties here, but quite often in my experience you need to position, pull and push elements based on grid elements: column and gutter. Based on dynamic variables not just something like 22.2% as you can always change basic grid options (by media queries, for example) and lose control over your blocks positioning.

@pepelsbey
Copy link
Author

@Snugug, could you please have a look at #184 pull request?

@Snugug
Copy link
Member

Snugug commented Jul 21, 2014

I thought I had commented on this.

Long story short: I'm uncomfortable changing the parameters of a mixin already in use as it would break existing code unnecessarily (@lolmaus, this is not the time or place to air your grievances about the needed changes with 1.2.x, please don't). This is something I'm happy to consider for Singularity 2.x, but I will not be considering it for 1.x, and I will not be releasing 2.x for just this feature; a major rewrite would need to happen.

To provide insight as to why the gutter-span mixin has different parameter order, it's because of what is being described. With gutter-span, the main actor is the gutter when using fluid grids and gutters, and as such, gutter is the first parameter. In grid-span, the main actors are the span and location, so they go first.

In the meantime, you can do the following to ease your specific issue:

gutter-span(null, $grid)

Generally with Singularity, and non-required properties in our mixins can have null passed into them and the default will be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants