{"action":"created","comment":{"url":"https://api.github.com/repos/elastic/eui/pulls/comments/194130696","pull_request_review_id":127252532,"id":194130696,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE5NDEzMDY5Ng==","diff_hunk":"@@ -665,19 +677,20 @@ export class EuiBasicTable extends Component {\n   renderItemFieldDataCell(itemId, item, column, columnIndex) {\n     const {\n       field,\n-      render,\n       textOnly,\n-      name, // eslint-disable-line no-unused-vars\n+      render,\n+      dataType,\n+      align,\n+      name,\n       description, // eslint-disable-line no-unused-vars\n-      dataType, // eslint-disable-line no-unused-vars\n       sortable, // eslint-disable-line no-unused-vars\n       ...rest\n     } = column;\n \n     const key = `_data_column_${field}_${itemId}_${columnIndex}`;\n-    const align = this.resolveColumnAlign(column);\n+    const columnAlign = align || this.getAlignForDataType(dataType);\n+    const contentRenderer = render || this.getRendererForDataType(dataType);","path":"src/components/basic_table/basic_table.js","position":103,"original_position":103,"commit_id":"bb3446dbe442329eae875d8e9bc49b7be380f593","original_commit_id":"bb3446dbe442329eae875d8e9bc49b7be380f593","user":{"login":"cjcenizal","id":1238659,"node_id":"MDQ6VXNlcjEyMzg2NTk=","avatar_url":"https://avatars2.githubusercontent.com/u/1238659?v=4","gravatar_id":"","url":"https://api.github.com/users/cjcenizal","html_url":"https://github.com/cjcenizal","followers_url":"https://api.github.com/users/cjcenizal/followers","following_url":"https://api.github.com/users/cjcenizal/following{/other_user}","gists_url":"https://api.github.com/users/cjcenizal/gists{/gist_id}","starred_url":"https://api.github.com/users/cjcenizal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cjcenizal/subscriptions","organizations_url":"https://api.github.com/users/cjcenizal/orgs","repos_url":"https://api.github.com/users/cjcenizal/repos","events_url":"https://api.github.com/users/cjcenizal/events{/privacy}","received_events_url":"https://api.github.com/users/cjcenizal/received_events","type":"User","site_admin":false},"body":"I was familiarizing myself with this code, and it took me awhile to track down how the column's `render` property was actually being consumed. I expected to see it called near where the cell itself was being rendered, but I had toi follow it through the original `resolveContentRenderer` method. This makes the connection more explicit and makes the new `getRendererForDataType` method simpler because it's only concerned with data types, not the entire column object.\r\n\r\nI applied this same pattern to the `resolveColumnAlign` method on the preceding line.","created_at":"2018-06-08T17:38:44Z","updated_at":"2018-06-08T17:38:45Z","html_url":"https://github.com/elastic/eui/pull/915#discussion_r194130696","pull_request_url":"https://api.github.com/repos/elastic/eui/pulls/915","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/elastic/eui/pulls/comments/194130696"},"html":{"href":"https://github.com/elastic/eui/pull/915#discussion_r194130696"},"pull_request":{"href":"https://api.github.com/repos/elastic/eui/pulls/915"}}},"pull_request":{"url":"https://api.github.com/repos/elastic/eui/pulls/915","id":193678354,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkzNjc4MzU0","html_url":"https://github.com/elastic/eui/pull/915","diff_url":"https://github.com/elastic/eui/pull/915.diff","patch_url":"https://github.com/elastic/eui/pull/915.patch","issue_url":"https://api.github.com/repos/elastic/eui/issues/915","number":915,"state":"open","locked":false,"title":"Refactor EuiInMemoryTable's cell rendering methods","user":{"login":"cjcenizal","id":1238659,"node_id":"MDQ6VXNlcjEyMzg2NTk=","avatar_url":"https://avatars2.githubusercontent.com/u/1238659?v=4","gravatar_id":"","url":"https://api.github.com/users/cjcenizal","html_url":"https://github.com/cjcenizal","followers_url":"https://api.github.com/users/cjcenizal/followers","following_url":"https://api.github.com/users/cjcenizal/following{/other_user}","gists_url":"https://api.github.com/users/cjcenizal/gists{/gist_id}","starred_url":"https://api.github.com/users/cjcenizal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cjcenizal/subscriptions","organizations_url":"https://api.github.com/users/cjcenizal/orgs","repos_url":"https://api.github.com/users/cjcenizal/repos","events_url":"https://api.github.com/users/cjcenizal/events{/privacy}","received_events_url":"https://api.github.com/users/cjcenizal/received_events","type":"User","site_admin":false},"body":"The goal of this PR is to reduce indirection and increase explicitness. I'll comment on the diff to explain some of the problems I encountered.","created_at":"2018-06-08T17:33:47Z","updated_at":"2018-06-08T17:38:45Z","closed_at":null,"merged_at":null,"merge_commit_sha":"90f4aadd1e670df3d3e7c6f8bc44e3a0ccb06ece","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/elastic/eui/pulls/915/commits","review_comments_url":"https://api.github.com/repos/elastic/eui/pulls/915/comments","review_comment_url":"https://api.github.com/repos/elastic/eui/pulls/comments{/number}","comments_url":"https://api.github.com/repos/elastic/eui/issues/915/comments","statuses_url":"https://api.github.com/repos/elastic/eui/statuses/bb3446dbe442329eae875d8e9bc49b7be380f593","head":{"label":"cjcenizal:refactor-in-memory-table","ref":"refactor-in-memory-table","sha":"bb3446dbe442329eae875d8e9bc49b7be380f593","user":{"login":"cjcenizal","id":1238659,"node_id":"MDQ6VXNlcjEyMzg2NTk=","avatar_url":"https://avatars2.githubusercontent.com/u/1238659?v=4","gravatar_id":"","url":"https://api.github.com/users/cjcenizal","html_url":"https://github.com/cjcenizal","followers_url":"https://api.github.com/users/cjcenizal/followers","following_url":"https://api.github.com/users/cjcenizal/following{/other_user}","gists_url":"https://api.github.com/users/cjcenizal/gists{/gist_id}","starred_url":"https://api.github.com/users/cjcenizal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cjcenizal/subscriptions","organizations_url":"https://api.github.com/users/cjcenizal/orgs","repos_url":"https://api.github.com/users/cjcenizal/repos","events_url":"https://api.github.com/users/cjcenizal/events{/privacy}","received_events_url":"https://api.github.com/users/cjcenizal/received_events","type":"User","site_admin":false},"repo":{"id":110029688,"node_id":"MDEwOlJlcG9zaXRvcnkxMTAwMjk2ODg=","name":"eui","full_name":"cjcenizal/eui","owner":{"login":"cjcenizal","id":1238659,"node_id":"MDQ6VXNlcjEyMzg2NTk=","avatar_url":"https://avatars2.githubusercontent.com/u/1238659?v=4","gravatar_id":"","url":"https://api.github.com/users/cjcenizal","html_url":"https://github.com/cjcenizal","followers_url":"https://api.github.com/users/cjcenizal/followers","following_url":"https://api.github.com/users/cjcenizal/following{/other_user}","gists_url":"https://api.github.com/users/cjcenizal/gists{/gist_id}","starred_url":"https://api.github.com/users/cjcenizal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cjcenizal/subscriptions","organizations_url":"https://api.github.com/users/cjcenizal/orgs","repos_url":"https://api.github.com/users/cjcenizal/repos","events_url":"https://api.github.com/users/cjcenizal/events{/privacy}","received_events_url":"https://api.github.com/users/cjcenizal/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/cjcenizal/eui","description":"Elastic UI Framework 🙌","fork":true,"url":"https://api.github.com/repos/cjcenizal/eui","forks_url":"https://api.github.com/repos/cjcenizal/eui/forks","keys_url":"https://api.github.com/repos/cjcenizal/eui/keys{/key_id}","collaborators_url":"https://api.github.com/repos/cjcenizal/eui/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/cjcenizal/eui/teams","hooks_url":"https://api.github.com/repos/cjcenizal/eui/hooks","issue_events_url":"https://api.github.com/repos/cjcenizal/eui/issues/events{/number}","events_url":"https://api.github.com/repos/cjcenizal/eui/events","assignees_url":"https://api.github.com/repos/cjcenizal/eui/assignees{/user}","branches_url":"https://api.github.com/repos/cjcenizal/eui/branches{/branch}","tags_url":"https://api.github.com/repos/cjcenizal/eui/tags","blobs_url":"https://api.github.com/repos/cjcenizal/eui/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/cjcenizal/eui/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/cjcenizal/eui/git/refs{/sha}","trees_url":"https://api.github.com/repos/cjcenizal/eui/git/trees{/sha}","statuses_url":"https://api.github.com/repos/cjcenizal/eui/statuses/{sha}","languages_url":"https://api.github.com/repos/cjcenizal/eui/languages","stargazers_url":"https://api.github.com/repos/cjcenizal/eui/stargazers","contributors_url":"https://api.github.com/repos/cjcenizal/eui/contributors","subscribers_url":"https://api.github.com/repos/cjcenizal/eui/subscribers","subscription_url":"https://api.github.com/repos/cjcenizal/eui/subscription","commits_url":"https://api.github.com/repos/cjcenizal/eui/commits{/sha}","git_commits_url":"https://api.github.com/repos/cjcenizal/eui/git/commits{/sha}","comments_url":"https://api.github.com/repos/cjcenizal/eui/comments{/number}","issue_comment_url":"https://api.github.com/repos/cjcenizal/eui/issues/comments{/number}","contents_url":"https://api.github.com/repos/cjcenizal/eui/contents/{+path}","compare_url":"https://api.github.com/repos/cjcenizal/eui/compare/{base}...{head}","merges_url":"https://api.github.com/repos/cjcenizal/eui/merges","archive_url":"https://api.github.com/repos/cjcenizal/eui/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/cjcenizal/eui/downloads","issues_url":"https://api.github.com/repos/cjcenizal/eui/issues{/number}","pulls_url":"https://api.github.com/repos/cjcenizal/eui/pulls{/number}","milestones_url":"https://api.github.com/repos/cjcenizal/eui/milestones{/number}","notifications_url":"https://api.github.com/repos/cjcenizal/eui/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/cjcenizal/eui/labels{/name}","releases_url":"https://api.github.com/repos/cjcenizal/eui/releases{/id}","deployments_url":"https://api.github.com/repos/cjcenizal/eui/deployments","created_at":"2017-11-08T20:57:33Z","updated_at":"2017-11-08T20:57:35Z","pushed_at":"2018-06-08T17:32:29Z","git_url":"git://github.com/cjcenizal/eui.git","ssh_url":"git@github.com:cjcenizal/eui.git","clone_url":"https://github.com/cjcenizal/eui.git","svn_url":"https://github.com/cjcenizal/eui","homepage":"https://elastic.github.io/eui/","size":56941,"stargazers_count":0,"watchers_count":0,"language":"JavaScript","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"elastic:master","ref":"master","sha":"5e0073ae536e11ef21954f0d5ca8c1ad1353c5ae","user":{"login":"elastic","id":6764390,"node_id":"MDEyOk9yZ2FuaXphdGlvbjY3NjQzOTA=","avatar_url":"https://avatars0.githubusercontent.com/u/6764390?v=4","gravatar_id":"","url":"https://api.github.com/users/elastic","html_url":"https://github.com/elastic","followers_url":"https://api.github.com/users/elastic/followers","following_url":"https://api.github.com/users/elastic/following{/other_user}","gists_url":"https://api.github.com/users/elastic/gists{/gist_id}","starred_url":"https://api.github.com/users/elastic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/elastic/subscriptions","organizations_url":"https://api.github.com/users/elastic/orgs","repos_url":"https://api.github.com/users/elastic/repos","events_url":"https://api.github.com/users/elastic/events{/privacy}","received_events_url":"https://api.github.com/users/elastic/received_events","type":"Organization","site_admin":false},"repo":{"id":107422373,"node_id":"MDEwOlJlcG9zaXRvcnkxMDc0MjIzNzM=","name":"eui","full_name":"elastic/eui","owner":{"login":"elastic","id":6764390,"node_id":"MDEyOk9yZ2FuaXphdGlvbjY3NjQzOTA=","avatar_url":"https://avatars0.githubusercontent.com/u/6764390?v=4","gravatar_id":"","url":"https://api.github.com/users/elastic","html_url":"https://github.com/elastic","followers_url":"https://api.github.com/users/elastic/followers","following_url":"https://api.github.com/users/elastic/following{/other_user}","gists_url":"https://api.github.com/users/elastic/gists{/gist_id}","starred_url":"https://api.github.com/users/elastic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/elastic/subscriptions","organizations_url":"https://api.github.com/users/elastic/orgs","repos_url":"https://api.github.com/users/elastic/repos","events_url":"https://api.github.com/users/elastic/events{/privacy}","received_events_url":"https://api.github.com/users/elastic/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/elastic/eui","description":"Elastic UI Framework 🙌","fork":false,"url":"https://api.github.com/repos/elastic/eui","forks_url":"https://api.github.com/repos/elastic/eui/forks","keys_url":"https://api.github.com/repos/elastic/eui/keys{/key_id}","collaborators_url":"https://api.github.com/repos/elastic/eui/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/elastic/eui/teams","hooks_url":"https://api.github.com/repos/elastic/eui/hooks","issue_events_url":"https://api.github.com/repos/elastic/eui/issues/events{/number}","events_url":"https://api.github.com/repos/elastic/eui/events","assignees_url":"https://api.github.com/repos/elastic/eui/assignees{/user}","branches_url":"https://api.github.com/repos/elastic/eui/branches{/branch}","tags_url":"https://api.github.com/repos/elastic/eui/tags","blobs_url":"https://api.github.com/repos/elastic/eui/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/elastic/eui/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/elastic/eui/git/refs{/sha}","trees_url":"https://api.github.com/repos/elastic/eui/git/trees{/sha}","statuses_url":"https://api.github.com/repos/elastic/eui/statuses/{sha}","languages_url":"https://api.github.com/repos/elastic/eui/languages","stargazers_url":"https://api.github.com/repos/elastic/eui/stargazers","contributors_url":"https://api.github.com/repos/elastic/eui/contributors","subscribers_url":"https://api.github.com/repos/elastic/eui/subscribers","subscription_url":"https://api.github.com/repos/elastic/eui/subscription","commits_url":"https://api.github.com/repos/elastic/eui/commits{/sha}","git_commits_url":"https://api.github.com/repos/elastic/eui/git/commits{/sha}","comments_url":"https://api.github.com/repos/elastic/eui/comments{/number}","issue_comment_url":"https://api.github.com/repos/elastic/eui/issues/comments{/number}","contents_url":"https://api.github.com/repos/elastic/eui/contents/{+path}","compare_url":"https://api.github.com/repos/elastic/eui/compare/{base}...{head}","merges_url":"https://api.github.com/repos/elastic/eui/merges","archive_url":"https://api.github.com/repos/elastic/eui/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/elastic/eui/downloads","issues_url":"https://api.github.com/repos/elastic/eui/issues{/number}","pulls_url":"https://api.github.com/repos/elastic/eui/pulls{/number}","milestones_url":"https://api.github.com/repos/elastic/eui/milestones{/number}","notifications_url":"https://api.github.com/repos/elastic/eui/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/elastic/eui/labels{/name}","releases_url":"https://api.github.com/repos/elastic/eui/releases{/id}","deployments_url":"https://api.github.com/repos/elastic/eui/deployments","created_at":"2017-10-18T14:49:20Z","updated_at":"2018-06-08T17:23:00Z","pushed_at":"2018-06-08T17:33:47Z","git_url":"git://github.com/elastic/eui.git","ssh_url":"git@github.com:elastic/eui.git","clone_url":"https://github.com/elastic/eui.git","svn_url":"https://github.com/elastic/eui","homepage":"https://elastic.github.io/eui/","size":80786,"stargazers_count":236,"watchers_count":236,"language":"JavaScript","has_issues":true,"has_projects":false,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":55,"mirror_url":null,"archived":false,"open_issues_count":151,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":55,"open_issues":151,"watchers":236,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/elastic/eui/pulls/915"},"html":{"href":"https://github.com/elastic/eui/pull/915"},"issue":{"href":"https://api.github.com/repos/elastic/eui/issues/915"},"comments":{"href":"https://api.github.com/repos/elastic/eui/issues/915/comments"},"review_comments":{"href":"https://api.github.com/repos/elastic/eui/pulls/915/comments"},"review_comment":{"href":"https://api.github.com/repos/elastic/eui/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/elastic/eui/pulls/915/commits"},"statuses":{"href":"https://api.github.com/repos/elastic/eui/statuses/bb3446dbe442329eae875d8e9bc49b7be380f593"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"cjcenizal"}}