From 5ef8582c3fd17efa1aa0df9498df2398bb81de1f Mon Sep 17 00:00:00 2001
From: CJ Cenizal <cj@cenizal.com>
Date: Thu, 24 May 2018 16:53:06 -0700
Subject: [PATCH] Add rowProps and cellProps props. Apply pass-through cell
 props directly to the td element instead of the child div.

---
 CHANGELOG.md                                  |  7 ++-
 src-docs/src/views/tables/basic/basic.js      | 27 ++++++---
 .../src/views/tables/basic/basic_section.js   |  9 +--
 .../__snapshots__/basic_table.test.js.snap    |  3 -
 src/components/basic_table/basic_table.js     | 57 +++++++++++++++----
 .../basic_table/basic_table.test.js           | 14 ++---
 .../__snapshots__/table_row_cell.test.js.snap |  4 +-
 src/components/table/table_row_cell.js        |  4 +-
 8 files changed, 84 insertions(+), 41 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b57b0a71564d..314129a1ca9a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,12 +1,15 @@
 ## [`master`](https://github.com/elastic/eui/tree/master)
 
-- `EuiBasicTable` and `EuiInMemoryTable` now let you define a `__props__` object on each row item,
-which lets you apply custom props to each row component ([#869](https://github.com/elastic/eui/pull/869))
+- `EuiBasicTable` and `EuiInMemoryTable` now accept `rowProps` and `cellProps` callbacks,
+which let you apply custom props to rows and props ([#869](https://github.com/elastic/eui/pull/869))
 
 **Breaking changes**
 
 - `EuiSearchBar` no longer has an `onParse` callback, and now passes an object to `onChange` with the shape `{ query, queryText, error }` ([#863](https://github.com/elastic/eui/pull/863))
 - `EuiInMemoryTable`'s `search.onChange` callback now passes an object with `{ query, queryText, error }` instead of only the query ([#863](https://github.com/elastic/eui/pull/863))
+- `EuiBasicTable` and `EuiInMemoryTable` pass-through cell props (defined by the `columns` prop
+and the `cellProps` prop) used to be applied to the `div` inside of the `td` element. They're
+now applied directly to the `td` element.  ([#869](https://github.com/elastic/eui/pull/869))
 
 **Bug fixes**
 
diff --git a/src-docs/src/views/tables/basic/basic.js b/src-docs/src/views/tables/basic/basic.js
index ba76e3399e6d..0eb966d078a0 100644
--- a/src-docs/src/views/tables/basic/basic.js
+++ b/src-docs/src/views/tables/basic/basic.js
@@ -80,22 +80,31 @@ export const Table = () => {
     }
   }];
 
-  const items = store.users.filter((user, index) => index < 10).map(user => {
-    const { id } = user;
+  const items = store.users.filter((user, index) => index < 10);
+
+  const getRowProps = (item) => {
+    const { id } = item;
+    return  {
+      'data-test-subj': `row-${id}`,
+      className: 'customRowClass',
+      onClick: () => console.log(`Clicked row ${id}`),
+    };
+  };
+
+  const getCellProps = (item, column, columnIndex) => {
+    const { id } = item;
     return {
-      ...user,
-      __props__: {
-        'data-test-subj': `row-${id}`,
-        className: 'customClass',
-        onClick: () => console.log(`Clicked row ${id}`),
-      },
+      className: 'customCellClass',
+      'data-test-subj': `cell-${id}-${columnIndex}`,
     };
-  });
+  };
 
   return (
     <EuiBasicTable
       items={items}
       columns={columns}
+      rowProps={getRowProps}
+      cellProps={getCellProps}
     />
   );
 };
diff --git a/src-docs/src/views/tables/basic/basic_section.js b/src-docs/src/views/tables/basic/basic_section.js
index 419cbfa3ec0b..4221067205b1 100644
--- a/src-docs/src/views/tables/basic/basic_section.js
+++ b/src-docs/src/views/tables/basic/basic_section.js
@@ -31,10 +31,11 @@ export const section = {
       <ul>
         <li>
           <EuiCode>items</EuiCode> are an array of objects that should be displayed in the table;
-          one item per row. You can define a <EuiCode>__props__</EuiCode> property on each item
-          object to define props to pass to the corresponding row component. The exact item data
-          that will be rendered in each cell in these rows is determined by
-          the <EuiCode>columns</EuiCode> property.
+          one item per row. The exact item data that will be rendered in each cell in these rows is
+          determined by the <EuiCode>columns</EuiCode> property.
+          You can define <EuiCode>rowProps</EuiCode> and <EuiCode>cellProps</EuiCode> props
+          which can either be objects and functions that return objects. The returned object&rsquo;s
+          will be applied as props to the rendered rows and row cells, respectively.
         </li>
         <li>
           <EuiCode>columns</EuiCode> defines what columns the table has and how to extract item data
diff --git a/src/components/basic_table/__snapshots__/basic_table.test.js.snap b/src/components/basic_table/__snapshots__/basic_table.test.js.snap
index 82553b8be8b3..9911f93856e6 100644
--- a/src/components/basic_table/__snapshots__/basic_table.test.js.snap
+++ b/src/components/basic_table/__snapshots__/basic_table.test.js.snap
@@ -226,10 +226,7 @@ exports[`EuiBasicTable items are rendered with custom props 1`] = `
           key="row_0"
         >
           <EuiTableRow
-            className="customClass"
-            data-test-subj="row"
             isSelected={false}
-            onClick={[Function]}
           >
             <EuiTableRowCell
               align="left"
diff --git a/src/components/basic_table/basic_table.js b/src/components/basic_table/basic_table.js
index 9918d8570ad6..1938df8d4011 100644
--- a/src/components/basic_table/basic_table.js
+++ b/src/components/basic_table/basic_table.js
@@ -118,7 +118,7 @@ export const ItemIdType = PropTypes.oneOfType([
 ]);
 
 export const SelectionType = PropTypes.shape({
-  onSelectionChange: PropTypes.func, // (selection: Record[]) => void;,
+  onSelectionChange: PropTypes.func, // (selection: item[]) => void;,
   selectable: PropTypes.func, // (item) => boolean;
   selectableMessage: PropTypes.func // (selectable, item) => boolean;
 });
@@ -144,11 +144,12 @@ const BasicTablePropTypes = {
   responsive: PropTypes.bool,
   isSelectable: PropTypes.bool,
   isExpandable: PropTypes.bool,
-  hasActions: PropTypes.bool
+  hasActions: PropTypes.bool,
+  rowProps: PropTypes.func,
+  cellProps: PropTypes.func,
 };
 
-export function getItemId(item, props) {
-  const { itemId } = props;
+export function getItemId(item, itemId) {
   if (itemId) {
     if (isFunction(itemId)) {
       return itemId(item);
@@ -157,8 +158,29 @@ export function getItemId(item, props) {
   }
 }
 
-export class EuiBasicTable extends Component {
+function getRowProps(item, rowProps) {
+  if (rowProps) {
+    if (isFunction(rowProps)) {
+      return rowProps(item);
+    }
+    return rowProps;
+  }
 
+  return {};
+}
+
+function getCellProps(item, column, columnIndex, cellProps) {
+  if (cellProps) {
+    if (isFunction(cellProps)) {
+      return cellProps(item, column, columnIndex);
+    }
+    return cellProps;
+  }
+
+  return {};
+}
+
+export class EuiBasicTable extends Component {
   static propTypes = BasicTablePropTypes;
   static defaultProps = {
     responsive: true,
@@ -171,8 +193,9 @@ export class EuiBasicTable extends Component {
       return { selection: [] };
     }
 
+    const { itemId } = nextProps;
     const selection = prevState.selection.filter(selectedItem => (
-      nextProps.items.findIndex(item => getItemId(item, nextProps) === getItemId(selectedItem, nextProps)) !== -1
+      nextProps.items.findIndex(item => getItemId(item, itemId) === getItemId(selectedItem, itemId)) !== -1
     ));
 
     return { selection };
@@ -280,6 +303,8 @@ export class EuiBasicTable extends Component {
       isSelectable, // eslint-disable-line no-unused-vars
       isExpandable, // eslint-disable-line no-unused-vars
       hasActions, // eslint-disable-line no-unused-vars
+      rowProps, // eslint-disable-line no-unused-vars
+      cellProps, // eslint-disable-line no-unused-vars
       ...rest
     } = this.props;
 
@@ -495,9 +520,10 @@ export class EuiBasicTable extends Component {
 
     const cells = [];
 
-    const itemId = getItemId(item, this.props) || rowIndex;
-    const selected = !selection ? false : this.state.selection && !!this.state.selection.find(selectedRecord => (
-      getItemId(selectedRecord, this.props) === itemId
+    const { itemId: itemIdCallback } = this.props;
+    const itemId = getItemId(item, itemIdCallback) || rowIndex;
+    const selected = !selection ? false : this.state.selection && !!this.state.selection.find(selectedItem => (
+      getItemId(selectedItem, itemIdCallback) === itemId
     ));
 
     if (selection) {
@@ -534,7 +560,8 @@ export class EuiBasicTable extends Component {
       </EuiTableRow>
     ) : undefined;
 
-    const { __props__: customRowProps } = item;
+    const { rowProps: rowPropsCallback } = this.props;
+    const rowProps = getRowProps(item, rowPropsCallback);
 
     return (
       <Fragment key={`row_${itemId}`}>
@@ -544,7 +571,7 @@ export class EuiBasicTable extends Component {
           isSelected={selected}
           hasActions={hasActions}
           isExpandable={isExpandable}
-          {...customRowProps}
+          {...rowProps}
         >
           {cells}
         </EuiTableRow>
@@ -563,8 +590,9 @@ export class EuiBasicTable extends Component {
       if (event.target.checked) {
         this.changeSelection([...this.state.selection, item]);
       } else {
+        const { itemId: itemIdCallback } = this.props;
         this.changeSelection(this.state.selection.reduce((selection, selectedItem) => {
-          if (getItemId(selectedItem, this.props) !== itemId) {
+          if (getItemId(selectedItem, itemIdCallback) !== itemId) {
             selection.push(selectedItem);
           }
           return selection;
@@ -651,6 +679,10 @@ export class EuiBasicTable extends Component {
     const value = get(item, field);
     const contentRenderer = this.resolveContentRenderer(column);
     const content = contentRenderer(value, item);
+
+    const { cellProps: cellPropsCallback } = this.props;
+    const cellProps = getCellProps(item, column, columnIndex, cellPropsCallback);
+
     return (
       <EuiTableRowCell
         key={key}
@@ -658,6 +690,7 @@ export class EuiBasicTable extends Component {
         header={column.name}
         // If there's no render function defined then we're only going to render text.
         textOnly={textOnly || !render}
+        {...cellProps}
         {...rest}
       >
         {content}
diff --git a/src/components/basic_table/basic_table.test.js b/src/components/basic_table/basic_table.test.js
index cfd754b45ede..9c4b16937fab 100644
--- a/src/components/basic_table/basic_table.test.js
+++ b/src/components/basic_table/basic_table.test.js
@@ -6,19 +6,19 @@ import { EuiBasicTable, getItemId } from './basic_table';
 
 describe('getItemId', () => {
   it('returns undefined if no itemId prop is given', () => {
-    expect(getItemId({ id: 5 }, {})).toBeUndefined();
-    expect(getItemId({ itemId: 5 }, {})).toBeUndefined();
-    expect(getItemId({ _itemId: 5 }, {})).toBeUndefined();
+    expect(getItemId({ id: 5 })).toBeUndefined();
+    expect(getItemId({ itemId: 5 })).toBeUndefined();
+    expect(getItemId({ _itemId: 5 })).toBeUndefined();
   });
 
   it('returns the correct id when a string itemId is given', () => {
-    expect(getItemId({ id: 5 }, { itemId: 'id' })).toBe(5);
-    expect(getItemId({ thing: '5' }, { itemId: 'thing' })).toBe('5');
+    expect(getItemId({ id: 5 }, 'id')).toBe(5);
+    expect(getItemId({ thing: '5' }, 'thing')).toBe('5');
   });
 
   it('returns the correct id when a function itemId is given', () => {
-    expect(getItemId({ id: 5 }, { itemId: () => 6 })).toBe(6);
-    expect(getItemId({ x: 2, y: 4 }, { itemId: ({ x, y }) => x * y })).toBe(8);
+    expect(getItemId({ id: 5 }, () => 6)).toBe(6);
+    expect(getItemId({ x: 2, y: 4 }, ({ x, y }) => x * y)).toBe(8);
   });
 });
 
diff --git a/src/components/table/__snapshots__/table_row_cell.test.js.snap b/src/components/table/__snapshots__/table_row_cell.test.js.snap
index d77cae1ddfb2..e7b5428c554c 100644
--- a/src/components/table/__snapshots__/table_row_cell.test.js.snap
+++ b/src/components/table/__snapshots__/table_row_cell.test.js.snap
@@ -58,12 +58,12 @@ exports[`children's className merges new classnames into existing ones 1`] = `
 
 exports[`renders EuiTableRowCell 1`] = `
 <td
+  aria-label="aria-label"
   class="euiTableRowCell"
+  data-test-subj="test subject string"
 >
   <div
-    aria-label="aria-label"
     class="euiTableCellContent testClass1 testClass2"
-    data-test-subj="test subject string"
   >
     <span
       class="euiTableCellContent__text"
diff --git a/src/components/table/table_row_cell.js b/src/components/table/table_row_cell.js
index 4a7d8271b49b..81ccce1ac01b 100644
--- a/src/components/table/table_row_cell.js
+++ b/src/components/table/table_row_cell.js
@@ -68,8 +68,8 @@ export const EuiTableRowCell = ({
   }
 
   return (
-    <td className={cellClasses} colSpan={colSpan} data-header={header}>
-      <div className={contentClasses} {...rest}>
+    <td className={cellClasses} colSpan={colSpan} data-header={header} {...rest}>
+      <div className={contentClasses}>
         {modifiedChildren}
       </div>
     </td>