Financial-Times / o-grid

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

Rhys/partial silencing2 #136

Closed wheresrhys closed 8 years ago

wheresrhys commented 8 years ago

@alicebartlett @AlbertoElias @onishiweb

Changes compared to the last PR (can't work in that branch as we consume it in prod)

wheresrhys commented 8 years ago

What do you think about the status of center/uncenter too? They're bundled in with human readable classes, but actually provide functionality with no equivalent in the column count based selectors

AlbertoElias commented 8 years ago

Pr looks good to me.

Re center, I don't mind if it stays behind the new if clause unless it's widely used, but I've never personally seen it

kaelig commented 8 years ago

These settings are kind of a footgun. They introduce the possibility that components aren't guaranteed to render well across environments, as grid classes might or might not be available to the browser on certain sites / apps. This could have a detrimental impact on code portability.

You might want to break the module in two, so that a component can declare it needs push/pull classes by loading an additional module. And you could remove the human friendly classes completey if they are not so useful.

alicebartlett commented 8 years ago

@kaelig FWIW I agree with you on the footgun, but Next argued strongly for this fucntionality. We'll be keeping an eye on it.

Origami has some work to do too, we'll be looking at who is looking at the human-friendly classes, and deprecating them if adoption is low enough.

wheresrhys commented 8 years ago

Perhaps it needs documenting better, but I see this functionality as being restricted in a similar way to silent mode toggling - for app use only. Components will still have access to mixins, which is all they're supposed to use anyway, and should be resilient to these toggles in the same way they're resilient to silent mode being on/off.

The problem with breaking in two is you then would have two namespaces (unless we introduce the notion of plugins into the origami spec, which has been discussed before).

I have no idea what a footgun is

triblondon commented 8 years ago

It assists in the process of shooting oneself in the foot. On Mon, 6 Jun 2016 at 19:36, Rhys Evans notifications@github.com wrote:

Perhaps it needs documenting better, but I see this functionality as being restricted in a similar way to silent mode toggling - for app use only. Components will still have access to mixins, which is all they're supposed to use anyway, and should be resilient to these toggles in the same way they're resilient to silent mode being on/off.

The problem with breaking in two is you then would have two namespaces (unless we introduce the notion of plugins into the origami spec, which has been discussed before).

I have no idea what a footgun is

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Financial-Times/o-grid/pull/136#issuecomment-223923889, or mute the thread https://github.com/notifications/unsubscribe/ABp630LfNwq0wd7t7ZOf-QVsWB-644gxks5qI_gngaJpZM4Itec4 .

wheresrhys commented 8 years ago

guns don't shoot feet, people shoot feet