Closed wheresrhys closed 9 years ago
@triblondon So this is ready to merge now. Is dropping ie7 down to single column gonna be contentious at all? @richard-still-ft
Should be fine. We possibly need to be slightly aware of unintended effects of people preferring the single column view on wide screens, whether it's easy for them to toggle it intentionally, and what that might do for ad views.
One other point I'd like to check - Is IE7 getting single col because it doesn't understand media queries? If so, wouldn't it get the default layout, rather than always the narrow one?
The default, non media query layout is a max-width 600px-ish.
Just realised - even in this layout multiple columns can be defined, so lack of box-sizing will still break the page in ie7. I may after all be necessary to revert the change that removed the ie7 boxsizing polyfill.
Definitely don't want to do that. I'd prefer to effectively enforce S12 as the non-media-query size of every cell. Mobile first.
That is a major change. Will probably result in a cleaner code base and smaller css file in the end, but will need careful QA. Do we have any automated image-diffing set up yet? Or a dedicated tester?
This change as it is is still a good step in clarifying and tidying. There's some breaking changes that need fixing to maintain backwards compatibility, then I think it can be merged, and we can move on to doing the larger change.
This should now be backwards compatible, and the fixed layout will only appear in ie8, but in ie7 it still won't display correctly as it is probably getting styles that it shouldn't be getting. Should these be wrapped in a media query? And if so, in this pr?
@wheresrhys as Andrew isn't here, do you mind reviewing please?
I don't think we need most of oGridOverrideDefaultGrid ()
any more
https://github.com/Financial-Times/o-grid/blob/ie7/src/scss/_utils.scss#L34 - rename to oGridTargetAtFixedLarge
I'll rename that and fix the other issued, and could you explain why the other function was needed?
it was to override all the stuff which now gets hidden within the media query, so I think we can safely remove
On 4 November 2014 12:23, Alberto Elias notifications@github.com wrote:
I'll rename that and fix the other issued, and could you explain why the other function was needed?
— Reply to this email directly or view it on GitHub https://github.com/Financial-Times/o-grid/pull/44#issuecomment-61631937.
This email was sent by a company owned by Pearson plc, registered office at 80 Strand, London WC2R 0RL. Registered in England and Wales with company number 53723.
Ok, I'll check and push a commit soon
@wheresrhys is that better?
Looks good.
@triblondon
I'm thinking of maybe removing the following too as they encourage non-responsive products:
$o-grid-fixed-layout-selector: null !default
overrides choice of useragents (normally ie8) with a custom selector$o-grid-is-fixed-desktop
: [false] Forces the site to always use the large layout