GitHub PullRequestReviewCommentEvent at 2014-10-20 08:40:27
Repository: h
Actor: aron
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/hypothesis/h/pulls/comments/19073248","id":19073248,"diff_hunk":"@@ -0,0 +1,45 @@\n+@import \"compass/utilities/general/clearfix\";\n+\n+// Mixins for working with grids. A grid consists of an outer grid container\n+// and internal columns. Each column has a gutter defined by $grid-gutter.\n+// The implementation is largely based on the grid system created by\n+// Harry Roberts <http://csswizardry.com/csswizardry-grids/>.\n+$grid-gutter: 20px;\n+\n+// Sets up styles for the grid wrapper.\n+@mixin grid-row {\n+  list-style: none;\n+  margin: 0;\n+  padding: 0;\n+  margin-left: -$grid-gutter; // Remove the margin from the first column.","path":"h/static/styles/mixins/grid.scss","position":14,"original_position":14,"commit_id":"8c46cd6daef0fb364b7a80bdae3d3c0d97f3be30","original_commit_id":"8c46cd6daef0fb364b7a80bdae3d3c0d97f3be30","user":{"login":"aron","id":47144,"avatar_url":"https://avatars.githubusercontent.com/u/47144?v=2","gravatar_id":"","url":"https://api.github.com/users/aron","html_url":"https://github.com/aron","followers_url":"https://api.github.com/users/aron/followers","following_url":"https://api.github.com/users/aron/following{/other_user}","gists_url":"https://api.github.com/users/aron/gists{/gist_id}","starred_url":"https://api.github.com/users/aron/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aron/subscriptions","organizations_url":"https://api.github.com/users/aron/orgs","repos_url":"https://api.github.com/users/aron/repos","events_url":"https://api.github.com/users/aron/events{/privacy}","received_events_url":"https://api.github.com/users/aron/received_events","type":"User","site_admin":false},"body":"In my mind the purpose of a grid system like this is to ensure the interface is organised consistently. Part of that is the spacing between content, which is where the gutter comes in. It may be that as we use this we find that different parts of the code require different grids, in which case we can move this variable into a mixin. For example the website will very likely use a different grid to the sidebar. An individual cell requiring a different padding should very much be the exception rather than the rule though.","created_at":"2014-10-20T08:40:27Z","updated_at":"2014-10-20T08:40:27Z","html_url":"https://github.com/hypothesis/h/pull/1577#discussion_r19073248","pull_request_url":"https://api.github.com/repos/hypothesis/h/pulls/1577","_links":{"self":{"href":"https://api.github.com/repos/hypothesis/h/pulls/comments/19073248"},"html":{"href":"https://github.com/hypothesis/h/pull/1577#discussion_r19073248"},"pull_request":{"href":"https://api.github.com/repos/hypothesis/h/pulls/1577"}}},"pull_request":{"url":"https://api.github.com/repos/hypothesis/h/pulls/1577","id":22918737,"html_url":"https://github.com/hypothesis/h/pull/1577","diff_url":"https://github.com/hypothesis/h/pull/1577.diff","patch_url":"https://github.com/hypothesis/h/pull/1577.patch","issue_url":"https://api.github.com/repos/hypothesis/h/issues/1577","number":1577,"state":"open","locked":false,"title":"Add a new simpler grid system","user":{"login":"aron","id":47144,"avatar_url":"https://avatars.githubusercontent.com/u/47144?v=2","gravatar_id":"","url":"https://api.github.com/users/aron","html_url":"https://github.com/aron","followers_url":"https://api.github.com/users/aron/followers","following_url":"https://api.github.com/users/aron/following{/other_user}","gists_url":"https://api.github.com/users/aron/gists{/gist_id}","starred_url":"https://api.github.com/users/aron/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aron/subscriptions","organizations_url":"https://api.github.com/users/aron/orgs","repos_url":"https://api.github.com/users/aron/repos","events_url":"https://api.github.com/users/aron/events{/privacy}","received_events_url":"https://api.github.com/users/aron/received_events","type":"User","site_admin":false},"body":"I've removed the YUI grid system, it wasn't being used anywhere and I wasn't a fan of the syntax. To me implementing a grid that worked across breakpoints seemed to require a large number of classes with each viewport bracket requiring it's own class.\r\n\r\nI've swapped it out for the onboarding page with a custom scss mixin that uses the same approach as the YUI system but uses friendlier class names and takes a mobile first approach. The idea being that you define the grid at the smallest viewport you need it and then tweak it as you need to as the viewport grows. I've used this approach before and like it a lot.\r\n\r\nI've also trimmed out most of the options to leave us with only what we need at the moment, a two, three column and four column grid. We can add more as necessary.\r\n\r\n```html\r\n<!-- gives single rows on mobile, a two column grid on tablets and four columns on desktops -->\r\n<div class=\"grid\">\r\n  <div class=\"column-tablet-one-half column-desktop-one-quarter\"></div>\r\n  <div class=\"column-tablet-one-half column-desktop-one-quarter\"></div>\r\n  <div class=\"column-tablet-one-half column-desktop-one-quarter\"></div>\r\n  <div class=\"column-tablet-one-half column-desktop-one-quarter\"></div>\r\n</div>\r\n```\r\n\r\nI also added some additional media queries to responsive.scss along the lines of `@include tablet-and-up {}`, moved file into the mixins directory and added some examples to the pattern library.","created_at":"2014-10-17T11:34:32Z","updated_at":"2014-10-20T08:40:27Z","closed_at":null,"merged_at":null,"merge_commit_sha":"295175d02d760560d374f2d263dad7d961f066f4","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/hypothesis/h/pulls/1577/commits","review_comments_url":"https://api.github.com/repos/hypothesis/h/pulls/1577/comments","review_comment_url":"https://api.github.com/repos/hypothesis/h/pulls/comments/{number}","comments_url":"https://api.github.com/repos/hypothesis/h/issues/1577/comments","statuses_url":"https://api.github.com/repos/hypothesis/h/statuses/8c46cd6daef0fb364b7a80bdae3d3c0d97f3be30","head":{"label":"hypothesis:grid-system","ref":"grid-system","sha":"8c46cd6daef0fb364b7a80bdae3d3c0d97f3be30","user":{"login":"hypothesis","id":1243215,"avatar_url":"https://avatars.githubusercontent.com/u/1243215?v=2","gravatar_id":"","url":"https://api.github.com/users/hypothesis","html_url":"https://github.com/hypothesis","followers_url":"https://api.github.com/users/hypothesis/followers","following_url":"https://api.github.com/users/hypothesis/following{/other_user}","gists_url":"https://api.github.com/users/hypothesis/gists{/gist_id}","starred_url":"https://api.github.com/users/hypothesis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hypothesis/subscriptions","organizations_url":"https://api.github.com/users/hypothesis/orgs","repos_url":"https://api.github.com/users/hypothesis/repos","events_url":"https://api.github.com/users/hypothesis/events{/privacy}","received_events_url":"https://api.github.com/users/hypothesis/received_events","type":"Organization","site_admin":false},"repo":{"id":3910945,"name":"h","full_name":"hypothesis/h","owner":{"login":"hypothesis","id":1243215,"avatar_url":"https://avatars.githubusercontent.com/u/1243215?v=2","gravatar_id":"","url":"https://api.github.com/users/hypothesis","html_url":"https://github.com/hypothesis","followers_url":"https://api.github.com/users/hypothesis/followers","following_url":"https://api.github.com/users/hypothesis/following{/other_user}","gists_url":"https://api.github.com/users/hypothesis/gists{/gist_id}","starred_url":"https://api.github.com/users/hypothesis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hypothesis/subscriptions","organizations_url":"https://api.github.com/users/hypothesis/orgs","repos_url":"https://api.github.com/users/hypothesis/repos","events_url":"https://api.github.com/users/hypothesis/events{/privacy}","received_events_url":"https://api.github.com/users/hypothesis/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/hypothesis/h","description":"The Internet, peer reviewed.","fork":false,"url":"https://api.github.com/repos/hypothesis/h","forks_url":"https://api.github.com/repos/hypothesis/h/forks","keys_url":"https://api.github.com/repos/hypothesis/h/keys{/key_id}","collaborators_url":"https://api.github.com/repos/hypothesis/h/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/hypothesis/h/teams","hooks_url":"https://api.github.com/repos/hypothesis/h/hooks","issue_events_url":"https://api.github.com/repos/hypothesis/h/issues/events{/number}","events_url":"https://api.github.com/repos/hypothesis/h/events","assignees_url":"https://api.github.com/repos/hypothesis/h/assignees{/user}","branches_url":"https://api.github.com/repos/hypothesis/h/branches{/branch}","tags_url":"https://api.github.com/repos/hypothesis/h/tags","blobs_url":"https://api.github.com/repos/hypothesis/h/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/hypothesis/h/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/hypothesis/h/git/refs{/sha}","trees_url":"https://api.github.com/repos/hypothesis/h/git/trees{/sha}","statuses_url":"https://api.github.com/repos/hypothesis/h/statuses/{sha}","languages_url":"https://api.github.com/repos/hypothesis/h/languages","stargazers_url":"https://api.github.com/repos/hypothesis/h/stargazers","contributors_url":"https://api.github.com/repos/hypothesis/h/contributors","subscribers_url":"https://api.github.com/repos/hypothesis/h/subscribers","subscription_url":"https://api.github.com/repos/hypothesis/h/subscription","commits_url":"https://api.github.com/repos/hypothesis/h/commits{/sha}","git_commits_url":"https://api.github.com/repos/hypothesis/h/git/commits{/sha}","comments_url":"https://api.github.com/repos/hypothesis/h/comments{/number}","issue_comment_url":"https://api.github.com/repos/hypothesis/h/issues/comments/{number}","contents_url":"https://api.github.com/repos/hypothesis/h/contents/{+path}","compare_url":"https://api.github.com/repos/hypothesis/h/compare/{base}...{head}","merges_url":"https://api.github.com/repos/hypothesis/h/merges","archive_url":"https://api.github.com/repos/hypothesis/h/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/hypothesis/h/downloads","issues_url":"https://api.github.com/repos/hypothesis/h/issues{/number}","pulls_url":"https://api.github.com/repos/hypothesis/h/pulls{/number}","milestones_url":"https://api.github.com/repos/hypothesis/h/milestones{/number}","notifications_url":"https://api.github.com/repos/hypothesis/h/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/hypothesis/h/labels{/name}","releases_url":"https://api.github.com/repos/hypothesis/h/releases{/id}","created_at":"2012-04-02T19:56:59Z","updated_at":"2014-10-15T19:35:54Z","pushed_at":"2014-10-20T08:07:59Z","git_url":"git://github.com/hypothesis/h.git","ssh_url":"git@github.com:hypothesis/h.git","clone_url":"https://github.com/hypothesis/h.git","svn_url":"https://github.com/hypothesis/h","homepage":"https://hypothes.is/","size":52134,"stargazers_count":329,"watchers_count":329,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":88,"mirror_url":null,"open_issues_count":130,"forks":88,"open_issues":130,"watchers":329,"default_branch":"master"}},"base":{"label":"hypothesis:master","ref":"master","sha":"fdaea5b99cb9b98a88fbe246dc58e1e043b557dd","user":{"login":"hypothesis","id":1243215,"avatar_url":"https://avatars.githubusercontent.com/u/1243215?v=2","gravatar_id":"","url":"https://api.github.com/users/hypothesis","html_url":"https://github.com/hypothesis","followers_url":"https://api.github.com/users/hypothesis/followers","following_url":"https://api.github.com/users/hypothesis/following{/other_user}","gists_url":"https://api.github.com/users/hypothesis/gists{/gist_id}","starred_url":"https://api.github.com/users/hypothesis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hypothesis/subscriptions","organizations_url":"https://api.github.com/users/hypothesis/orgs","repos_url":"https://api.github.com/users/hypothesis/repos","events_url":"https://api.github.com/users/hypothesis/events{/privacy}","received_events_url":"https://api.github.com/users/hypothesis/received_events","type":"Organization","site_admin":false},"repo":{"id":3910945,"name":"h","full_name":"hypothesis/h","owner":{"login":"hypothesis","id":1243215,"avatar_url":"https://avatars.githubusercontent.com/u/1243215?v=2","gravatar_id":"","url":"https://api.github.com/users/hypothesis","html_url":"https://github.com/hypothesis","followers_url":"https://api.github.com/users/hypothesis/followers","following_url":"https://api.github.com/users/hypothesis/following{/other_user}","gists_url":"https://api.github.com/users/hypothesis/gists{/gist_id}","starred_url":"https://api.github.com/users/hypothesis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hypothesis/subscriptions","organizations_url":"https://api.github.com/users/hypothesis/orgs","repos_url":"https://api.github.com/users/hypothesis/repos","events_url":"https://api.github.com/users/hypothesis/events{/privacy}","received_events_url":"https://api.github.com/users/hypothesis/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/hypothesis/h","description":"The Internet, peer reviewed.","fork":false,"url":"https://api.github.com/repos/hypothesis/h","forks_url":"https://api.github.com/repos/hypothesis/h/forks","keys_url":"https://api.github.com/repos/hypothesis/h/keys{/key_id}","collaborators_url":"https://api.github.com/repos/hypothesis/h/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/hypothesis/h/teams","hooks_url":"https://api.github.com/repos/hypothesis/h/hooks","issue_events_url":"https://api.github.com/repos/hypothesis/h/issues/events{/number}","events_url":"https://api.github.com/repos/hypothesis/h/events","assignees_url":"https://api.github.com/repos/hypothesis/h/assignees{/user}","branches_url":"https://api.github.com/repos/hypothesis/h/branches{/branch}","tags_url":"https://api.github.com/repos/hypothesis/h/tags","blobs_url":"https://api.github.com/repos/hypothesis/h/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/hypothesis/h/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/hypothesis/h/git/refs{/sha}","trees_url":"https://api.github.com/repos/hypothesis/h/git/trees{/sha}","statuses_url":"https://api.github.com/repos/hypothesis/h/statuses/{sha}","languages_url":"https://api.github.com/repos/hypothesis/h/languages","stargazers_url":"https://api.github.com/repos/hypothesis/h/stargazers","contributors_url":"https://api.github.com/repos/hypothesis/h/contributors","subscribers_url":"https://api.github.com/repos/hypothesis/h/subscribers","subscription_url":"https://api.github.com/repos/hypothesis/h/subscription","commits_url":"https://api.github.com/repos/hypothesis/h/commits{/sha}","git_commits_url":"https://api.github.com/repos/hypothesis/h/git/commits{/sha}","comments_url":"https://api.github.com/repos/hypothesis/h/comments{/number}","issue_comment_url":"https://api.github.com/repos/hypothesis/h/issues/comments/{number}","contents_url":"https://api.github.com/repos/hypothesis/h/contents/{+path}","compare_url":"https://api.github.com/repos/hypothesis/h/compare/{base}...{head}","merges_url":"https://api.github.com/repos/hypothesis/h/merges","archive_url":"https://api.github.com/repos/hypothesis/h/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/hypothesis/h/downloads","issues_url":"https://api.github.com/repos/hypothesis/h/issues{/number}","pulls_url":"https://api.github.com/repos/hypothesis/h/pulls{/number}","milestones_url":"https://api.github.com/repos/hypothesis/h/milestones{/number}","notifications_url":"https://api.github.com/repos/hypothesis/h/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/hypothesis/h/labels{/name}","releases_url":"https://api.github.com/repos/hypothesis/h/releases{/id}","created_at":"2012-04-02T19:56:59Z","updated_at":"2014-10-15T19:35:54Z","pushed_at":"2014-10-20T08:07:59Z","git_url":"git://github.com/hypothesis/h.git","ssh_url":"git@github.com:hypothesis/h.git","clone_url":"https://github.com/hypothesis/h.git","svn_url":"https://github.com/hypothesis/h","homepage":"https://hypothes.is/","size":52134,"stargazers_count":329,"watchers_count":329,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":88,"mirror_url":null,"open_issues_count":130,"forks":88,"open_issues":130,"watchers":329,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/hypothesis/h/pulls/1577"},"html":{"href":"https://github.com/hypothesis/h/pull/1577"},"issue":{"href":"https://api.github.com/repos/hypothesis/h/issues/1577"},"comments":{"href":"https://api.github.com/repos/hypothesis/h/issues/1577/comments"},"review_comments":{"href":"https://api.github.com/repos/hypothesis/h/pulls/1577/comments"},"review_comment":{"href":"https://api.github.com/repos/hypothesis/h/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/hypothesis/h/pulls/1577/commits"},"statuses":{"href":"https://api.github.com/repos/hypothesis/h/statuses/8c46cd6daef0fb364b7a80bdae3d3c0d97f3be30"}}}}