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

Add optional parameters to grid mixins for more flexibility #6230

Closed
wants to merge 6 commits into from

Conversation

chimericdream
Copy link

In some responsive designs, the number of columns can change at different breakpoints. In addition, the helper mixins for creating rows and columns (.makeRow() and its cousins) can produce incorrect values based on changing grid and column widths. The code in this pull request fixes these issues and allows for more flexibility when using the various grid-related mixins.

#grid > .core, #grid > .fluid, #grid > .input
Although these mixins allow for customizing the column and gutter widths (e.g. to have a different sized grid inside a media query), they all default to the same number of grid columns. This makes sense as a default option, but the number of columns should be configurable as well. An optional parameter allows both methods: by default the grid uses the same number of columns all the time, but the developer can override the column count in their LESS file if it makes sense to do so.

Example of current method:

@gridColumns:     12;    //
@gridColumnWidth: 60px;  // Standard values for 960px grid
@gridGutterWidth: 20px;  //

@gridColumnWidth320: 20px;
@gridGutterWidth320: 6px;

#grid > .core(@gridColumnWidth, @gridGutterWidth); // Generates a standard 960px grid with 12 columns

@media all and (min-width: 320px) and (max-width: 479px) {
    #grid > .core(@gridColumnWidth320, @gridGutterWidth320); // Generates a 12-column grid 312px wide
}

The above example produces an oddly sized grid. A better option is to reduce the number of columns to allow for a cleaner overall look.

Same example with configurable columns:

@gridColumns:     12;    //
@gridColumnWidth: 60px;  // Standard values for 960px grid
@gridGutterWidth: 20px;  //

@gridColumns320:     8;
@gridColumnWidth320: 30px;
@gridGutterWidth320: 10px;

#grid > .core(@gridColumnWidth, @gridGutterWidth); // Generates a standard 960px grid with 12 columns

@media all and (min-width: 320px) and (max-width: 479px) {
    #grid > .core(@gridColumnWidth320, @gridGutterWidth320, @gridColumns320); // Generates an 8-column grid 320px wide
}

Note: The examples for #grid > .fluid and #grid > .input are essentially the same as above, so I am leaving them out for the sake of simplicity.


.makeRow(), .makeColumn(), .tableColumns()
The way these mixins are currently set up, they default to the standard grid size (i.e. column & gutter widths), regardless where they are used. This means that it is not possible to use .makeRow() inside a media query that has a different width. Even without the changes above, this can result in odd behavior.

Example of current method:

@gridColumns:     12;    //
@gridColumnWidth: 60px;  // Standard values for 960px grid
@gridGutterWidth: 20px;  //

@gridColumnWidth720: 45px;
@gridGutterWidth720: 15px;

#grid > .core(@gridColumnWidth, @gridGutterWidth); // Generates a standard 960px grid with 12 columns

.some-container {
    .makeRow(); // sets margin-left: -20px;
}

@media all and (min-width: 720px) and (max-width: 959px) {
    #grid > .core(@gridColumnWidth720, @gridGutterWidth720); // Generates a 12-column grid 720px wide

    .some-container {
        .makeRow(); // sets margin-left: -20px;
    }
}

Obviously, the call to .makeRow() in the media query sets the container's margin incorrectly. Things get worse with .makeColumn() and .tableColumns().

Example of current method:

@gridColumns:     12;    //
@gridColumnWidth: 60px;  // Standard values for 960px grid
@gridGutterWidth: 20px;  //

@gridColumnWidth720: 45px;
@gridGutterWidth720: 15px;

#grid > .core(@gridColumnWidth, @gridGutterWidth); // Generates a standard 960px grid with 12 columns

.some-container {
    .makeRow();

    nav {
        .makeColumn(3);
    }
    article {
        .makeColumn(6);
    }
    aside {
        .makeColumn(3);
    }
}

@media all and (min-width: 720px) and (max-width: 959px) {
    #grid > .core(@gridColumnWidth720, @gridGutterWidth720); // Generates a 12-column grid 720px wide

    .some-container {
        .makeRow();

        nav {
            .makeColumn(3);
        }
        article {
            .makeColumn(6);
        }
        aside {
            .makeColumn(3);
        }
    }
}

This is the compiled result of the above (note: I have removed the stuff coming from .clearfix(), the .container, and .(span|offset)X for readability):

.some-container {
  margin-left: -20px;
  *zoom: 1;
}

.some-container nav {
  float: left;
  margin-left: 20px;
  width: 220px;
}

.some-container article {
  float: left;
  margin-left: 20px;
  width: 460px;
}

.some-container aside {
  float: left;
  margin-left: 20px;
  width: 220px;
}

@media all and (min-width: 720px) and (max-width: 959px) {
  .some-container {
    margin-left: -20px; /* Incorrect */
    *zoom: 1;
  }

  .some-container nav {
    float: left;
    margin-left: 20px;  /* Incorrect */
    width: 220px;       /* Incorrect */
  }

  .some-container article {
    float: left;
    margin-left: 20px;  /* Incorrect */
    width: 460px;       /* Incorrect */
  }

  .some-container aside {
    float: left;
    margin-left: 20px;  /* Incorrect */
    width: 220px;       /* Incorrect */
  }
}

This is what it should be, based on the different column and gutter widths used in the 720px grid:

.some-container {
  margin-left: -20px;
  *zoom: 1;
}

.some-container nav {
  float: left;
  margin-left: 20px;
  width: 220px;
}

.some-container article {
  float: left;
  margin-left: 20px;
  width: 460px;
}

.some-container aside {
  float: left;
  margin-left: 20px;
  width: 220px;
}

@media all and (min-width: 720px) and (max-width: 959px) {
  .some-container {
    margin-left: -15px;
    *zoom: 1;
  }

  .some-container nav {
    float: left;
    margin-left: 15px;
    width: 165px;
  }

  .some-container article {
    float: left;
    margin-left: 15px;
    width: 345px;
  }

  .some-container aside {
    float: left;
    margin-left: 15px;
    width: 165px;
  }
}

Note: As with the first examples for #grid > .(core|fluid|input)(), the examples for .makeColumn() and .tableColumns() are essentially the same, so .tableColumns() has been omitted for readability.

…trary number of columns. Defaults to the value of @gridColumns to preserve current functionality.
…itrary number of columns. Defaults to the value of @gridColumns to preserve current functionality.
…itrary number of columns. Defaults to the value of @gridColumns to preserve current functionality.
…ifferent grid sizes (e.g. in responsive layouts). Defaults to the value of @gridGutterWidth to preserve current functionality.
…lumn() to allow for different grid sizes (e.g. in responsive layouts). Defaults to the values of @gridColumnWidth and @gridGutterWidth, respectively, to preserve current functionality.
…olumns() to allow for different grid sizes (e.g. in responsive layouts). Defaults to the values of @gridColumnWidth and @gridGutterWidth, respectively, to preserve current functionality.
@cescalante
Copy link

+1

@mdo
Copy link
Member

mdo commented Dec 20, 2012

Amazing pull request and explanation, but for now we're going to punt on these changes as 3.0 really overhauls the grid. We'll need to play with it some there before integrating further changes like this. Thanks though!

I'd also encourage you to check out the 3.0 pull request, #6342, to see these new changes and what's upcoming. Perhaps we can work in some more grid enhancements in smaller chunks.

@mdo mdo closed this Dec 20, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants