Financial-Times / o-grid

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

Implement gutter overrides using classes #25

Closed wheresrhys closed 9 years ago

wheresrhys commented 10 years ago

Currently gutters can only be disabled on a column by extending placeholder classes, an approach taken only because encapsulating this in classes would have added a great deal of complexity, both to the implementation details and the readability of the class.

But to keep this feature consistent with column sizing and switching on/off of silent selectors it makes sense to now have a data-o-grid-gutters attribute alongside these placeholder classes

Syntax ideas

All the below implement two cases:

triblondon commented 10 years ago

I vote for uppercase for reasons already discussed. I don't like the pipes and dots - it's not at all obvious which way around it is. 1/0 works but feels like we can do better. The square bracket syntax works reasonably well for me.

data-o-grid-gutters=".] [M. .L."
wheresrhys commented 10 years ago

A few other ideas

data-o-grid-gutters=".: :M. .L."
data-o-grid-gutters=":| |M: :L:"
data-o-grid-gutters="._ _M. .L."
data-o-grid-gutters="-= =M- -L-"

The last is my favourite by far

triblondon commented 10 years ago

I preferred the square bracket syntax, but could we compromise on:

data-o-grid-gutters="-+ +M- -L-"

+ for more gutter, - for less gutter, seems a little more semantic.

wheresrhys commented 10 years ago

I like that a lot. It also leaves room (should it be required in future) to use = to mean default/inherit

triblondon commented 10 years ago

Great. Is it finished yet? :-)

triblondon commented 9 years ago

Can I resurrect this thread? I think we were overcomplicating the issue - most of the time I just want to be able to stick a class on a cell to remove either the left or right gutter, so could we define some concrete classes for this?

o-grid-no-gutters[-S/M/L/XL]
o-grid-no-left-gutter[-S/M/L/XL]
o-grid-no-right-gutter[-S/M/L/XL]
wheresrhys commented 9 years ago

Yep, sensible idea. I think there was a bit of data attribute mania going about a few months ago, as the other approaches suggested above look totally mad to me now.

kaelig commented 9 years ago

We may want to use a slightly more BEM-ish syntax?

This is just a suggestion:

// for all breakpoints
.o-grid-remove-gutters {}
.o-grid-remove-gutters--left {}
.o-grid-remove-gutters--right {}

// for a specific breakpoint
.o-grid-remove-gutters--[S/M/L/XL] {}
.o-grid-remove-gutters--[S/M/L/XL]--left {}
.o-grid-remove-gutters--[S/M/L/XL]--right {}

Not sure about the position of the breakpoint modifier in the class name, though.

wheresrhys commented 9 years ago

BEM-ish is good for sure.

I prefer the breakpoint modifier last as it feels like the final modification, and the one most likely to be altered in future.

I'm trying to think of a positive, non-composite term instead of no/remove-gutters, but 'gutterless' is the best I've got, which is no improvement really.

wheresrhys commented 9 years ago

Just remembered also that a row can be put into compact mode so that all child columns are gutterless. So it may be worth adding classes (and placeholders) to add gutters, in which case I think add-/remove-gutters is a good choice of wording in the class.

kaelig commented 9 years ago

In that case we'd have something like this?

// for all breakpoints
// for all breakpoints
.o-grid-add-gutters {}
.o-grid-add-gutters--left {}
.o-grid-add-gutters--right {}
.o-grid-remove-gutters {}
.o-grid-remove-gutters--left {}
.o-grid-remove-gutters--right {}

// for a specific breakpoint
.o-grid-add-gutters--[S/M/L/XL] {}
.o-grid-add-gutters--left--[S/M/L/XL] {}
.o-grid-add-gutters--right--[S/M/L/XL] {}
.o-grid-remove-gutters--[S/M/L/XL] {}
.o-grid-remove-gutters--left--[S/M/L/XL] {}
.o-grid-remove-gutters--right--[S/M/L/XL] {}
wheresrhys commented 9 years ago

yep, which is starting to remind me why I didn't include concrete classes in the first place, as the output in non-silent mode is fairly long-winded.

AlbertoElias commented 9 years ago

BTW, we are now going to start trying to move from placeholder classes to mixins to avoid this issue

triblondon commented 9 years ago

Agree with https://github.com/Financial-Times/o-grid/issues/25#issuecomment-58651795. Rhys, could you implement. Also note that we're now aggressively avoiding placeholders, and should try to gradually convert those that we have into mixins, but in this case since the placeholders already exist, I think we're only talking about adding equivalent concrete classes.

kaelig commented 9 years ago

@triblondon: we're now aggressively avoiding placeholders

Glad to hear that. in my experience @extend % should be allowed within a module itself, but not between modules.

AlbertoElias commented 9 years ago

But if you're using compact mode, why would you want to add gutters? Just checking to make sure it's absolute necessary to be able to add gutters

wheresrhys commented 9 years ago

there was a use case taht inspired considering it - can't remember what it was now. Maybe a part of the fastft design

AlbertoElias commented 9 years ago

It's very simple to add, so I won't until the need is confirmed.

AlbertoElias commented 9 years ago

I'm going to close this while removing gutters isn't necessary