Financial-Times / o-grid

Responsive grid system
http://registry.origami.ft.com/components/o-grid
93 stars 14 forks source link

oGridAddLayout broken? #111

Closed wheresrhys closed 8 years ago

wheresrhys commented 8 years ago

The following works

$o-grid-is-silent: false;
$o-grid-layouts: (
    S:   490px,
    M:   740px,
    L:   980px,
    XL:  1220px,
    XXL: 1600px
);
@import 'o-grid/main';

But this (which should do the same, only without having to redefine the default sizes) appears to do nothing

@import 'o-grid/main';
@include oGridAddLayout(
     $layout-name: XXL,
     $layout-width: 1600px
);
@include oGridSurfaceCurrentLayout;
@include oGridGenerate;
wheresrhys commented 8 years ago

As an aside, it'd be great if the following shorthand for the above were supported

@import 'o-grid/main';
@include oGridAddLayout(
     $layout-name: XXL,
     $layout-width: 1600px,
     $generate: true
);

... though not sure it's feasible

kaelig commented 8 years ago

Looks like adding a large layout (larger than the rest) fails.

Interestingly enough, there is no unit test for this use case.

A good first step would be to add a test case here: https://github.com/Financial-Times/o-grid/blob/master/test/

AlbertoElias commented 8 years ago

Fixed in #119

kaelig commented 8 years ago

Well done!