{"action":"closed","number":1497,"pull_request":{"url":"https://api.github.com/repos/NewSpring/Holtzman/pulls/1497","id":94902437,"html_url":"https://github.com/NewSpring/Holtzman/pull/1497","diff_url":"https://github.com/NewSpring/Holtzman/pull/1497.diff","patch_url":"https://github.com/NewSpring/Holtzman/pull/1497.patch","issue_url":"https://api.github.com/repos/NewSpring/Holtzman/issues/1497","number":1497,"state":"closed","locked":false,"title":"Styling Components Research","user":{"login":"JakeDawkins","id":9259509,"avatar_url":"https://avatars.githubusercontent.com/u/9259509?v=3","gravatar_id":"","url":"https://api.github.com/users/JakeDawkins","html_url":"https://github.com/JakeDawkins","followers_url":"https://api.github.com/users/JakeDawkins/followers","following_url":"https://api.github.com/users/JakeDawkins/following{/other_user}","gists_url":"https://api.github.com/users/JakeDawkins/gists{/gist_id}","starred_url":"https://api.github.com/users/JakeDawkins/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JakeDawkins/subscriptions","organizations_url":"https://api.github.com/users/JakeDawkins/orgs","repos_url":"https://api.github.com/users/JakeDawkins/repos","events_url":"https://api.github.com/users/JakeDawkins/events{/privacy}","received_events_url":"https://api.github.com/users/JakeDawkins/received_events","type":"User","site_admin":false},"body":"## tldr;\r\n\r\nI refactored\r\n`imports/components/icons/Accounts.js`\r\n`imports/components/accountType/index.js`\r\n`imports/blocks/discover/Item.js` (incomplete)\r\n\r\nIn this PR, I only show comparisons of our current solution (Aphrodite) and the proposed solution (styled-components).\r\n\r\nAphrodite creates a single `<style>` block at the top of the page for styles. At runtime, the `css()` function generates a classname for each component. It does not have support for react-native, which is an issue. It does not support global CSS or all of the CSS spec. It does not support theming, and overriding isn't very straight-forward.\r\n\r\nStyled-Components supports react-native (which is a great plus). It injects styles similar to how aphrodite does, but it supports all of the CSS spec as well as global CSS styles. It would take a bit of refactoring to convert to, though.\r\n\r\n**Edit: This is possible, actually. Haven't done it yet.**\r\n\r\nIn the PR, I changed discover cards to use this (mostly) instead of aphrodite, and some things, like passing a new class to ImageLoader for making the images fill the space don't work. We would need to rethink how we pass that kind of customizability in. With aphrodite, we were doing: \r\n\r\n```\r\n  bgClasses = () => (\r\n    [\r\n      \"grid__item\",\r\n      \"two-fifths\",\r\n      \"hard\",\r\n      \"soft-half-left\",\r\n      \"background--cover\",\r\n      css(Styles[\"height-100\"]),\r\n    ]\r\n```\r\n\r\nand I changed it to:\r\n\r\n```\r\n  bgClasses = `\r\n    grid__item\r\n    two-fifths\r\n    hard\r\n    soft-half-left\r\n    background--cover\r\n  `;\r\n```\r\n\r\n<img width=\"697\" alt=\"screen shot 2016-11-22 at 4 23 42 pm\" src=\"https://cloud.githubusercontent.com/assets/9259509/20542789/2929b10a-b0d1-11e6-95a0-3042e2305b1b.png\">\r\n\r\n\r\n## Aphrodite:\r\n\r\n> CSS is only injected into the <head> of the page when these classes are used. The injection happens in Aphrodite's css() function\r\n\r\n* [why only a single `style` component](https://github.com/Khan/aphrodite/issues/45)\r\n\r\n* no support for react native\r\n\r\n* could abstract styles to a separate file by importing the `styles` object\r\n\r\n* [Allows extension of classes](https://github.com/Khan/aphrodite#user-content-advanced-extensions), although they may break because of specificity\r\n\r\n## styled-components:\r\n\r\n* [Video explaining styled-components](https://youtu.be/19gqsBc_Cx0?t=29m00s). The beginning of the video compares other styling solutions.\r\n\r\n* [Does support SSR](https://github.com/styled-components/styled-components/issues/124) as do all the other options.\r\n\r\n* Supports react-native styling (with actual CSS) :party:\r\n\r\n* Supports all valid CSS (including @font-face and @keyframe) \r\n\r\n* Uses [Glamor](https://github.com/threepointone/glamor) for style injection.\r\n\r\n* Enforces style declarations outside of JSX, keeping render methods clean.\r\n\r\n* [Has Flow type support](https://github.com/styled-components/styled-components/blob/master/docs/flow-support.md).\r\n\r\n* A bit verbose, and involves creating a new component and caling that component.\r\n\r\n## Conclusion\r\n\r\nstyled-components could be beneficial to our long-term development. As we eventually move Holzmann to react-native, we will need to use some solution to style our components, and being able to use the same styling language for both native and web components would make our work quite a bit quicker and less painful.\r\n\r\nTheming might be extremely useful down the road, as we allow and assist other churches to use holzmann (or even when we use it for Fuse). It would allow us to define things like theme colors, borders, etc in one place making the change quick and easy.\r\n\r\n**To figure out**\r\n\r\nI'm not sure _yet_ how using styled-components will affect testing of components. I imagine it would at most require us exporting the component before being wrapped (which we already do with some components).\r\n\r\nI'm also not certain what kind of problems we will run into with our existing CSS codebase and specificity? \r\n\r\nSome cases of aphrodite use will not be possible to exactly replicate. We'll have to rethink how we do it.","created_at":"2016-11-22T21:34:13Z","updated_at":"2016-12-11T13:45:10Z","closed_at":"2016-12-11T13:45:10Z","merged_at":null,"merge_commit_sha":"5174cd8cfc5d8fd8c684e1d93744cba99390145b","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/NewSpring/Holtzman/pulls/1497/commits","review_comments_url":"https://api.github.com/repos/NewSpring/Holtzman/pulls/1497/comments","review_comment_url":"https://api.github.com/repos/NewSpring/Holtzman/pulls/comments{/number}","comments_url":"https://api.github.com/repos/NewSpring/Holtzman/issues/1497/comments","statuses_url":"https://api.github.com/repos/NewSpring/Holtzman/statuses/868606bd7d2e043cb4c38f399f2463806968e02a","head":{"label":"NewSpring:try-styled-components","ref":"try-styled-components","sha":"868606bd7d2e043cb4c38f399f2463806968e02a","user":{"login":"NewSpring","id":1241964,"avatar_url":"https://avatars.githubusercontent.com/u/1241964?v=3","gravatar_id":"","url":"https://api.github.com/users/NewSpring","html_url":"https://github.com/NewSpring","followers_url":"https://api.github.com/users/NewSpring/followers","following_url":"https://api.github.com/users/NewSpring/following{/other_user}","gists_url":"https://api.github.com/users/NewSpring/gists{/gist_id}","starred_url":"https://api.github.com/users/NewSpring/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NewSpring/subscriptions","organizations_url":"https://api.github.com/users/NewSpring/orgs","repos_url":"https://api.github.com/users/NewSpring/repos","events_url":"https://api.github.com/users/NewSpring/events{/privacy}","received_events_url":"https://api.github.com/users/NewSpring/received_events","type":"Organization","site_admin":false},"repo":{"id":31171598,"name":"Holtzman","full_name":"NewSpring/Holtzman","owner":{"login":"NewSpring","id":1241964,"avatar_url":"https://avatars.githubusercontent.com/u/1241964?v=3","gravatar_id":"","url":"https://api.github.com/users/NewSpring","html_url":"https://github.com/NewSpring","followers_url":"https://api.github.com/users/NewSpring/followers","following_url":"https://api.github.com/users/NewSpring/following{/other_user}","gists_url":"https://api.github.com/users/NewSpring/gists{/gist_id}","starred_url":"https://api.github.com/users/NewSpring/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NewSpring/subscriptions","organizations_url":"https://api.github.com/users/NewSpring/orgs","repos_url":"https://api.github.com/users/NewSpring/repos","events_url":"https://api.github.com/users/NewSpring/events{/privacy}","received_events_url":"https://api.github.com/users/NewSpring/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/NewSpring/Holtzman","description":"The main NewSpring namespace and utilities for all newspring packages. ","fork":false,"url":"https://api.github.com/repos/NewSpring/Holtzman","forks_url":"https://api.github.com/repos/NewSpring/Holtzman/forks","keys_url":"https://api.github.com/repos/NewSpring/Holtzman/keys{/key_id}","collaborators_url":"https://api.github.com/repos/NewSpring/Holtzman/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/NewSpring/Holtzman/teams","hooks_url":"https://api.github.com/repos/NewSpring/Holtzman/hooks","issue_events_url":"https://api.github.com/repos/NewSpring/Holtzman/issues/events{/number}","events_url":"https://api.github.com/repos/NewSpring/Holtzman/events","assignees_url":"https://api.github.com/repos/NewSpring/Holtzman/assignees{/user}","branches_url":"https://api.github.com/repos/NewSpring/Holtzman/branches{/branch}","tags_url":"https://api.github.com/repos/NewSpring/Holtzman/tags","blobs_url":"https://api.github.com/repos/NewSpring/Holtzman/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/NewSpring/Holtzman/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/NewSpring/Holtzman/git/refs{/sha}","trees_url":"https://api.github.com/repos/NewSpring/Holtzman/git/trees{/sha}","statuses_url":"https://api.github.com/repos/NewSpring/Holtzman/statuses/{sha}","languages_url":"https://api.github.com/repos/NewSpring/Holtzman/languages","stargazers_url":"https://api.github.com/repos/NewSpring/Holtzman/stargazers","contributors_url":"https://api.github.com/repos/NewSpring/Holtzman/contributors","subscribers_url":"https://api.github.com/repos/NewSpring/Holtzman/subscribers","subscription_url":"https://api.github.com/repos/NewSpring/Holtzman/subscription","commits_url":"https://api.github.com/repos/NewSpring/Holtzman/commits{/sha}","git_commits_url":"https://api.github.com/repos/NewSpring/Holtzman/git/commits{/sha}","comments_url":"https://api.github.com/repos/NewSpring/Holtzman/comments{/number}","issue_comment_url":"https://api.github.com/repos/NewSpring/Holtzman/issues/comments{/number}","contents_url":"https://api.github.com/repos/NewSpring/Holtzman/contents/{+path}","compare_url":"https://api.github.com/repos/NewSpring/Holtzman/compare/{base}...{head}","merges_url":"https://api.github.com/repos/NewSpring/Holtzman/merges","archive_url":"https://api.github.com/repos/NewSpring/Holtzman/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/NewSpring/Holtzman/downloads","issues_url":"https://api.github.com/repos/NewSpring/Holtzman/issues{/number}","pulls_url":"https://api.github.com/repos/NewSpring/Holtzman/pulls{/number}","milestones_url":"https://api.github.com/repos/NewSpring/Holtzman/milestones{/number}","notifications_url":"https://api.github.com/repos/NewSpring/Holtzman/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/NewSpring/Holtzman/labels{/name}","releases_url":"https://api.github.com/repos/NewSpring/Holtzman/releases{/id}","deployments_url":"https://api.github.com/repos/NewSpring/Holtzman/deployments","created_at":"2015-02-22T17:27:05Z","updated_at":"2016-12-05T14:39:52Z","pushed_at":"2016-12-11T03:56:04Z","git_url":"git://github.com/NewSpring/Holtzman.git","ssh_url":"git@github.com:NewSpring/Holtzman.git","clone_url":"https://github.com/NewSpring/Holtzman.git","svn_url":"https://github.com/NewSpring/Holtzman","homepage":"http://holtzman.newspring.io/","size":27888,"stargazers_count":12,"watchers_count":12,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":5,"mirror_url":null,"open_issues_count":133,"forks":5,"open_issues":133,"watchers":12,"default_branch":"master"}},"base":{"label":"NewSpring:master","ref":"master","sha":"726ba471519777e376768690019cd0f1abe6fcfe","user":{"login":"NewSpring","id":1241964,"avatar_url":"https://avatars.githubusercontent.com/u/1241964?v=3","gravatar_id":"","url":"https://api.github.com/users/NewSpring","html_url":"https://github.com/NewSpring","followers_url":"https://api.github.com/users/NewSpring/followers","following_url":"https://api.github.com/users/NewSpring/following{/other_user}","gists_url":"https://api.github.com/users/NewSpring/gists{/gist_id}","starred_url":"https://api.github.com/users/NewSpring/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NewSpring/subscriptions","organizations_url":"https://api.github.com/users/NewSpring/orgs","repos_url":"https://api.github.com/users/NewSpring/repos","events_url":"https://api.github.com/users/NewSpring/events{/privacy}","received_events_url":"https://api.github.com/users/NewSpring/received_events","type":"Organization","site_admin":false},"repo":{"id":31171598,"name":"Holtzman","full_name":"NewSpring/Holtzman","owner":{"login":"NewSpring","id":1241964,"avatar_url":"https://avatars.githubusercontent.com/u/1241964?v=3","gravatar_id":"","url":"https://api.github.com/users/NewSpring","html_url":"https://github.com/NewSpring","followers_url":"https://api.github.com/users/NewSpring/followers","following_url":"https://api.github.com/users/NewSpring/following{/other_user}","gists_url":"https://api.github.com/users/NewSpring/gists{/gist_id}","starred_url":"https://api.github.com/users/NewSpring/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NewSpring/subscriptions","organizations_url":"https://api.github.com/users/NewSpring/orgs","repos_url":"https://api.github.com/users/NewSpring/repos","events_url":"https://api.github.com/users/NewSpring/events{/privacy}","received_events_url":"https://api.github.com/users/NewSpring/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/NewSpring/Holtzman","description":"The main NewSpring namespace and utilities for all newspring packages. ","fork":false,"url":"https://api.github.com/repos/NewSpring/Holtzman","forks_url":"https://api.github.com/repos/NewSpring/Holtzman/forks","keys_url":"https://api.github.com/repos/NewSpring/Holtzman/keys{/key_id}","collaborators_url":"https://api.github.com/repos/NewSpring/Holtzman/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/NewSpring/Holtzman/teams","hooks_url":"https://api.github.com/repos/NewSpring/Holtzman/hooks","issue_events_url":"https://api.github.com/repos/NewSpring/Holtzman/issues/events{/number}","events_url":"https://api.github.com/repos/NewSpring/Holtzman/events","assignees_url":"https://api.github.com/repos/NewSpring/Holtzman/assignees{/user}","branches_url":"https://api.github.com/repos/NewSpring/Holtzman/branches{/branch}","tags_url":"https://api.github.com/repos/NewSpring/Holtzman/tags","blobs_url":"https://api.github.com/repos/NewSpring/Holtzman/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/NewSpring/Holtzman/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/NewSpring/Holtzman/git/refs{/sha}","trees_url":"https://api.github.com/repos/NewSpring/Holtzman/git/trees{/sha}","statuses_url":"https://api.github.com/repos/NewSpring/Holtzman/statuses/{sha}","languages_url":"https://api.github.com/repos/NewSpring/Holtzman/languages","stargazers_url":"https://api.github.com/repos/NewSpring/Holtzman/stargazers","contributors_url":"https://api.github.com/repos/NewSpring/Holtzman/contributors","subscribers_url":"https://api.github.com/repos/NewSpring/Holtzman/subscribers","subscription_url":"https://api.github.com/repos/NewSpring/Holtzman/subscription","commits_url":"https://api.github.com/repos/NewSpring/Holtzman/commits{/sha}","git_commits_url":"https://api.github.com/repos/NewSpring/Holtzman/git/commits{/sha}","comments_url":"https://api.github.com/repos/NewSpring/Holtzman/comments{/number}","issue_comment_url":"https://api.github.com/repos/NewSpring/Holtzman/issues/comments{/number}","contents_url":"https://api.github.com/repos/NewSpring/Holtzman/contents/{+path}","compare_url":"https://api.github.com/repos/NewSpring/Holtzman/compare/{base}...{head}","merges_url":"https://api.github.com/repos/NewSpring/Holtzman/merges","archive_url":"https://api.github.com/repos/NewSpring/Holtzman/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/NewSpring/Holtzman/downloads","issues_url":"https://api.github.com/repos/NewSpring/Holtzman/issues{/number}","pulls_url":"https://api.github.com/repos/NewSpring/Holtzman/pulls{/number}","milestones_url":"https://api.github.com/repos/NewSpring/Holtzman/milestones{/number}","notifications_url":"https://api.github.com/repos/NewSpring/Holtzman/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/NewSpring/Holtzman/labels{/name}","releases_url":"https://api.github.com/repos/NewSpring/Holtzman/releases{/id}","deployments_url":"https://api.github.com/repos/NewSpring/Holtzman/deployments","created_at":"2015-02-22T17:27:05Z","updated_at":"2016-12-05T14:39:52Z","pushed_at":"2016-12-11T03:56:04Z","git_url":"git://github.com/NewSpring/Holtzman.git","ssh_url":"git@github.com:NewSpring/Holtzman.git","clone_url":"https://github.com/NewSpring/Holtzman.git","svn_url":"https://github.com/NewSpring/Holtzman","homepage":"http://holtzman.newspring.io/","size":27888,"stargazers_count":12,"watchers_count":12,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":5,"mirror_url":null,"open_issues_count":133,"forks":5,"open_issues":133,"watchers":12,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/NewSpring/Holtzman/pulls/1497"},"html":{"href":"https://github.com/NewSpring/Holtzman/pull/1497"},"issue":{"href":"https://api.github.com/repos/NewSpring/Holtzman/issues/1497"},"comments":{"href":"https://api.github.com/repos/NewSpring/Holtzman/issues/1497/comments"},"review_comments":{"href":"https://api.github.com/repos/NewSpring/Holtzman/pulls/1497/comments"},"review_comment":{"href":"https://api.github.com/repos/NewSpring/Holtzman/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/NewSpring/Holtzman/pulls/1497/commits"},"statuses":{"href":"https://api.github.com/repos/NewSpring/Holtzman/statuses/868606bd7d2e043cb4c38f399f2463806968e02a"}},"merged":false,"mergeable":false,"mergeable_state":"dirty","merged_by":null,"comments":5,"review_comments":0,"commits":3,"additions":249,"deletions":467,"changed_files":19}}
{"actor":{"display_login":"jbaxleyiii"}}