Foundation-sites: [Foundation for Sites] Error in grid-context mixin

[ikemarrus]

How can we reproduce this bug?
In version 6.2.1
1.set a grid context of 10 (or another number other than the default) with nested scss that calls grid-column to set up columns - the column width is correctly calculated based on 10 columns

1. Outside that nested scss, call grid-column to set up columns. look at how widths for columns are being calculated - they are still being calculated with a grid context of 10 instead of the default grid context (e.g. 12)

I looked at the code for grid-context:

$old-grid-column-count: $grid-column-count;
$grid-column-count: $columns !global;
@if $root {
@at-root {
@content;
}
}
@else {
@content;
}
// Restore the old column count
$grid-column-count: $old-grid-column-count;

Note that $grid-column-count is specified as a global variable when it's being set, but as a local variable when it's being reset. When I changed the reset to also be global, the grid-context was restored correctly

[andycochran]

Is this still an issue in v6.2.3?

[Baedda]

@andycochran yes it is. I dont know why, but there was merged a pull request in december 2015:
https://github.com/zurb/foundation-sites/commit/56d819af094a41e20c21726224a16cbd005ce668

In my opionion this pull request is responsible for the _false_ behaviour. Take a look to what the description of the pr says:
https://github.com/zurb/foundation-sites/pull/7538
The old behaviour was correct. Using the grid-row-mixin should not effect the default $grid-column-count. I think this change has to be reverted. Otherwise the default-variable doesnt make any sense. If you have a default grid with 10 columns and switch only one row to have e.g. 15 columns then every following row will be calculated with 15 instead of 10 columns.

In the example @ocularrhythm didn´t use the row-mixin correctly. Looking to the sourcecode of the grid-row-mixin you have to nest the styling that should be effected by a column-count change inside the row-mixin (@content):

if $columns != null {
  @include grid-context($columns) {
    @content;
  }
}

In this ticket (https://github.com/zurb/foundation-sites/issues/7498) @gakimball has described how to use the grid-row mixin to get the desired effect. But the pr was nevertheless merged into the core.

I can fix this on my side by using the grid-contetxt-mixin at each use but this is really annoying :/
Hope there will be a fix soon.

[andycochran]

@Baedda Do we just need to add !global back into that variable?

[Baedda]

@andycochran in my opinion: yes.
Take a look ... i did a small test case.

$grid-column-count: 10;

.row-a {
    @include grid-row;
    .half-width-column {
        @include grid-column(5);
        @debug $grid-column-count;
    }
}

.row-b {
    @include grid-row(12) {
        .half-width-column {
            @include grid-column(6);
            @debug $grid-column-count;
        }
    }
}

.row-c {
    @include grid-row;
    .half-width-column {
        @include grid-column(5);
        @debug $grid-column-count;
    }
}

With the current version of Foundation the debugs do print out the following column counts to the console:

/.../src/styles/app.scss:31 DEBUG: 10
/.../src/styles/app.scss:39 DEBUG: 12
/.../src/styles/app.scss:47 DEBUG: 12

For me this seems not like the wanted behaviour. .row-c should use the value configured via $grid-column-count. When I open up the _row.scss from foundation and add the !global back to that variable then the output is this:

/.../src/styles/app.scss:31 DEBUG: 10
/.../src/styles/app.scss:39 DEBUG: 12
/.../src/styles/app.scss:47 DEBUG: 10

And this is exactly what I would expect from configuring the _main_ grid to have 10 columns.

[andycochran]

Makes sense. Let's put !global back in there. It'd also be nice to have tests for this.

@Baedda, can you make PR?

@brettsmason, do you have any feedback?

[brettsmason]

@andycochran I've read over everything and I agree, looks like the logical thing to do is to add it back.
A test would be good to ensure this doesn't reoccur too.

[Baedda]

Here is the PR: https://github.com/zurb/foundation-sites/pull/9055

[studiobovenkamer]

I've tested the #9055 commit and this works like expected for me.

[michaelworm]

Can this be merged quickly please? We had a hard time finding out why the heck all of our grid-columns got wrong widths after using grid-row Mixin and overriding the column gutter for a specific case. The docs currently lead to a hard to find error ... http://foundation.zurb.com/sites/docs/grid.html#multiple-grids

[kball]

Merged #9055. This will be out in the 6.2.4 release, should be in the next few weeks.

[rogercoathup]

Assuming this also fixes / closes: https://github.com/zurb/foundation-sites/issues/9198