Financial-Times / o-grid

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

Conditional comments #10

Closed wheresrhys closed 10 years ago

wheresrhys commented 10 years ago

You're advising people to add conditional comments to their products. Currently Origami's syntax guidelines for HTML disagree with that, so we should take another look at that. Your conditional comment examples also contain no-js classes, which are presumably not a grid concern. There's nothing to suggest that a product would have any javascript to remove those classes. @triblondon

wheresrhys commented 10 years ago

ie7/8 are the only supported desktop browsers which lack MQ support so to me it seems like overkill to feature detect in order to serve an experience only intended for these two fixed targets. And conditional comments are the accepted way to target ie7/8 (e.g. html5boilerplate used them right up until it dropped ie7/8 support). I agree with your point about no-js classes though. Perhaps if origami docs contain a recommendation on which classes should be added to html and how, which includes how to target ie7 & 8, then the grid docs could just say they follow the origami convention and link to that section

triblondon commented 10 years ago

Since grid makes that option configurable, why not simply say that to use grid, the product developer must either specify a selector to target browsers that are to get a fixed width layout, add the classes that grid expects by default (which by convention you would do using conditional comments, but that's an exercise for the product developer), or accept that grid in their product will be responsive on all browsers.

wheresrhys commented 10 years ago

Can't say I agree with origami being agnostic on how to target ie7/8 but it makes sense to be less prescriptive in grid-module. Will update docs in next version

triblondon commented 10 years ago

OK. If you want this in Origami can you open an origami issue and we'll discuss next week.