{"action":"created","issue":{"url":"https://api.github.com/repos/alphagov/finder-frontend/issues/160","labels_url":"https://api.github.com/repos/alphagov/finder-frontend/issues/160/labels{/name}","comments_url":"https://api.github.com/repos/alphagov/finder-frontend/issues/160/comments","events_url":"https://api.github.com/repos/alphagov/finder-frontend/issues/160/events","html_url":"https://github.com/alphagov/finder-frontend/pull/160","id":56420346,"number":160,"title":"Use new grid layout","user":{"login":"dsingleton","id":63201,"avatar_url":"https://avatars.githubusercontent.com/u/63201?v=3","gravatar_id":"","url":"https://api.github.com/users/dsingleton","html_url":"https://github.com/dsingleton","followers_url":"https://api.github.com/users/dsingleton/followers","following_url":"https://api.github.com/users/dsingleton/following{/other_user}","gists_url":"https://api.github.com/users/dsingleton/gists{/gist_id}","starred_url":"https://api.github.com/users/dsingleton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dsingleton/subscriptions","organizations_url":"https://api.github.com/users/dsingleton/orgs","repos_url":"https://api.github.com/users/dsingleton/repos","events_url":"https://api.github.com/users/dsingleton/events{/privacy}","received_events_url":"https://api.github.com/users/dsingleton/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-02-03T18:56:41Z","updated_at":"2015-02-04T10:24:45Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/alphagov/finder-frontend/pulls/160","html_url":"https://github.com/alphagov/finder-frontend/pull/160","diff_url":"https://github.com/alphagov/finder-frontend/pull/160.diff","patch_url":"https://github.com/alphagov/finder-frontend/pull/160.patch"},"body":"There a significant visual difference, so i'm not sure this is ready to merge, but I think theres enough to be worth discussion. I'd recommend looking at the commits (and their messages) in isolation, rather than the whole diff.\r\n\r\nMostly this converted fine, there were some sticking points around:\r\n- Needing an element to wrap the filters and results columns, acting as a grid row - basically not the `header`. I chose `.filtering` as a not-awful start, but I think we could do better.\r\n- Continuing to enforce the max-width of the `header`, to ensure the line length doesn't go beyond `2/3`rds, is done in a fiddly way. It requires floating \r\n - One option would be to it in a row/column, but that would add quite a bit of markup just for CSS hooks.\r\n - Another would be using max-width \r\n- Finally, the show stopper; the gutters between columns is significantly smaller, `30px` in the new vs `60px` in the old. As the screenshots show, it's a hefty change.\r\n\r\nThe first point is semantic flddling, while the other two are a class of problem we'll see on other applications when trying to use modern grids and so are worth proper discussion.\r\n\r\nScreenshots!\r\n\r\n\r\n| Before | After |\r\n| ---------| ------- |\r\n| ![finder-before](https://cloud.githubusercontent.com/assets/63201/6026775/527b5b24-abd5-11e4-86e8-7344585397cd.png) | ![finder-after](https://cloud.githubusercontent.com/assets/63201/6026774/527abb7e-abd5-11e4-959d-c0233483828c.png) | \r\n| ![finder-desktop-before](https://cloud.githubusercontent.com/assets/63201/6026779/52806506-abd5-11e4-81e9-1e3184906a13.png) | ![screenshot 2015-02-04 10 18 56](https://cloud.githubusercontent.com/assets/63201/6038709/60f32d3a-ac57-11e4-977b-26991b4ac8c5.png) | \r\n| ![finder-email-before](https://cloud.githubusercontent.com/assets/63201/6026776/527c8788-abd5-11e4-96f7-c4fec4d9a298.png) | ![finder-email-after](https://cloud.githubusercontent.com/assets/63201/6026778/527f6282-abd5-11e4-8a70-3fb996a3f356.png) | \r\n| ![finder-email-desktop-before](https://cloud.githubusercontent.com/assets/63201/6026780/528e7f6a-abd5-11e4-8304-ec6839482cdc.png) | ![finder-email-desktop-after](https://cloud.githubusercontent.com/assets/63201/6026781/528ee450-abd5-11e4-812f-4952bb55ce29.png) |"},"comment":{"url":"https://api.github.com/repos/alphagov/finder-frontend/issues/comments/72831798","html_url":"https://github.com/alphagov/finder-frontend/pull/160#issuecomment-72831798","issue_url":"https://api.github.com/repos/alphagov/finder-frontend/issues/160","id":72831798,"user":{"login":"futurefabric","id":265403,"avatar_url":"https://avatars.githubusercontent.com/u/265403?v=3","gravatar_id":"","url":"https://api.github.com/users/futurefabric","html_url":"https://github.com/futurefabric","followers_url":"https://api.github.com/users/futurefabric/followers","following_url":"https://api.github.com/users/futurefabric/following{/other_user}","gists_url":"https://api.github.com/users/futurefabric/gists{/gist_id}","starred_url":"https://api.github.com/users/futurefabric/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/futurefabric/subscriptions","organizations_url":"https://api.github.com/users/futurefabric/orgs","repos_url":"https://api.github.com/users/futurefabric/repos","events_url":"https://api.github.com/users/futurefabric/events{/privacy}","received_events_url":"https://api.github.com/users/futurefabric/received_events","type":"User","site_admin":false},"created_at":"2015-02-04T10:24:45Z","updated_at":"2015-02-04T10:24:45Z","body":"There's less visual separation with that tighter gutter, but I don't think this should prevent merging. \r\nWe can always take a look at it separately later if needs be."}}