search

bbc / grandstand

BBC Grandstand is a collection of common CSS abstractions and utility helper classes
http://bit.ly/grandstandcss
MIT License
191 stars 9 forks source link

Adding Objects - Keylines, Links #54

Closed sareh closed 5 years ago

sareh commented 8 years ago

The change to responsive-image is because we want to a) not load images by default on core b) only load images on core if they have a '--lead' class. We compromised, since we wanted to initally modify gs-o-responsive-image to have display: none; on core & display: none; on enhanced, but realised this would break current functionality for everyone using this class currently. That is why we added the (admittedly not-well-named) class gs-o-responsive-image--enhanced-only. Any suggestions would be welcome!

sareh commented 8 years ago

@shaunbent @mintuz What do you think? Especially about the updates to existing object patterns.

wildlyinaccurate commented 8 years ago

Probably worth noting that the keyline Sass produces quite a bit of CSS. We could optimise it a fair bit if we split it into separate keyline/spacing classes like gs-o-keyline and gs-o-keyline__spacing+

sareh commented 8 years ago

OK, to minimise the size further, should we remove the spacing variations completely? So that we just use gs-o-keyline & gs-o-no-keyline for horizontal and gs-o-keyline-vertical & gs-o-no-keyline-vertical

Then, if anyone wants spacing, they can use the gs-u-mb gs-u-mb+ gs-u-mb++ classes in conjunction with these?

Just an example use-case makes it look a bit complicated, though :/ e.g. gs-o-keyline gs-u-mb+ gs-o-no-keyline@m gs-u-mb0@m

wildlyinaccurate commented 8 years ago

Not quite -- the margin & padding utility classes apply to the selected element but keylines are on an ::after element.

sareh commented 8 years ago

Oh yes, of course! /o\

sareh commented 8 years ago

@wildlyinaccurate What do you think of the changes to keyline.scss?

wildlyinaccurate commented 8 years ago

+1 for removing the mixin -- I think that made them more confusing and (to my surprise) didn't help Sass to optimise the CSS at all.

I'm not sure about the keyline class changes though. The convention everywhere else in Grandstand is that a "plus" class is double($gel-spacing-unit) and "plusplus" is quadruple($gel-spacing-unit). I think making the keylines different confuses things. It also gives the impression that having zero padding is the norm, when actually we only use the zero padding variant once.

sareh commented 8 years ago

OK, I thought that having a gs-o-no-keyline and a gs-o-keyline0 would be confusing, but yes, consistency with the rest of grandstand makes more sense. :)

wildlyinaccurate commented 8 years ago

To be fair, the whole thing is confusing. My first thought when I see keyline+ is "oh it'll be a double-width keyline". 😕