elastic · uboness · Mar 19, 2018 · Feb 26, 2018
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 **Bug fixes**
 
 - Made `EuiIconTip` screen reader accessible ([#534](https://github.com/elastic/eui/pull/534))
+- Fixed a sorting issue in `EuiInMemoryTable` ([#453](https://github.com/elastic/eui/pull/453))
 
 # [`0.0.30`](https://github.com/elastic/eui/tree/v0.0.30)
 

diff --git a/src/components/basic_table/__snapshots__/basic_table.test.js.snap b/src/components/basic_table/__snapshots__/basic_table.test.js.snap
@@ -8,8 +8,6 @@ exports[`EuiBasicTable basic - empty - custom message 1`] = `
     <EuiTableHeader>
       <EuiTableHeaderCell
         align="left"
-        isSortAscending={false}
-        isSorted={false}
         key="_data_h_name_0"
         scope="col"
       >
@@ -39,8 +37,6 @@ exports[`EuiBasicTable basic - empty - custom message as node 1`] = `
     <EuiTableHeader>
       <EuiTableHeaderCell
         align="left"
-        isSortAscending={false}
-        isSorted={false}
         key="_data_h_name_0"
         scope="col"
       >
@@ -78,8 +74,6 @@ exports[`EuiBasicTable basic - empty 1`] = `
     <EuiTableHeader>
       <EuiTableHeaderCell
         align="left"
-        isSortAscending={false}
-        isSorted={false}
         key="_data_h_name_0"
         scope="col"
       >
@@ -109,8 +103,6 @@ exports[`EuiBasicTable basic - with items 1`] = `
     <EuiTableHeader>
       <EuiTableHeaderCell
         align="left"
-        isSortAscending={false}
-        isSorted={false}
         key="_data_h_name_0"
         scope="col"
       >
@@ -173,8 +165,6 @@ exports[`EuiBasicTable with pagination - 2nd page 1`] = `
     <EuiTableHeader>
       <EuiTableHeaderCell
         align="left"
-        isSortAscending={false}
-        isSorted={false}
         key="_data_h_name_0"
         scope="col"
       >
@@ -234,8 +224,6 @@ exports[`EuiBasicTable with pagination 1`] = `
     <EuiTableHeader>
       <EuiTableHeaderCell
         align="left"
-        isSortAscending={false}
-        isSorted={false}
         key="_data_h_name_0"
         scope="col"
       >
@@ -309,8 +297,6 @@ exports[`EuiBasicTable with pagination and error 1`] = `
     <EuiTableHeader>
       <EuiTableHeaderCell
         align="left"
-        isSortAscending={false}
-        isSorted={false}
         key="_data_h_name_0"
         scope="col"
       >
@@ -360,8 +346,6 @@ exports[`EuiBasicTable with pagination and selection 1`] = `
       </EuiTableHeaderCellCheckbox>
       <EuiTableHeaderCell
         align="left"
-        isSortAscending={false}
-        isSorted={false}
         key="_data_h_name_0"
         scope="col"
       >
@@ -1389,6 +1373,68 @@ exports[`EuiBasicTable with pagination, selection, sorting, column renderer and
 </div>
 `;
 
+exports[`EuiBasicTable with sortable columns and sorting disabled 1`] = `
+<div
+  className="euiBasicTable testClass1 testClass2"
+>
+  <EuiTable>
+    <EuiTableHeader>
+      <EuiTableHeaderCell
+        align="left"
+        key="_data_h_name_0"
+        scope="col"
+      >
+        Name
+      </EuiTableHeaderCell>
+    </EuiTableHeader>
+    <EuiTableBody>
+      <EuiTableRow
+        isSelected={false}
+        key="row_0_0"
+        onMouseOut={[Function]}
+        onMouseOver={[Function]}
+      >
+        <EuiTableRowCell
+          align="left"
+          key="_data_column_name_0_0"
+          textOnly={true}
+        >
+          name1
+        </EuiTableRowCell>
+      </EuiTableRow>
+      <EuiTableRow
+        isSelected={false}
+        key="row_1_1"
+        onMouseOut={[Function]}
+        onMouseOver={[Function]}
+      >
+        <EuiTableRowCell
+          align="left"
+          key="_data_column_name_1_0"
+          textOnly={true}
+        >
+          name2
+        </EuiTableRowCell>
+      </EuiTableRow>
+      <EuiTableRow
+        isSelected={false}
+        key="row_2_2"
+        onMouseOut={[Function]}
+        onMouseOver={[Function]}
+      >
+        <EuiTableRowCell
+          align="left"
+          key="_data_column_name_2_0"
+          textOnly={true}
+        >
+          name3
+        </EuiTableRowCell>
+      </EuiTableRow>
+    </EuiTableBody>
+  </EuiTable>
+</div>
+`;
+
 exports[`EuiBasicTable with sorting 1`] = `
 <div
   className="euiBasicTable testClass1 testClass2"

diff --git a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap
@@ -335,6 +335,11 @@ exports[`EuiInMemoryTable with pagination, selection and sorting 1`] = `
       "onSelectionChanged": [Function],
     }
   }
+  sorting={
+    Object {
+      "sort": undefined,
+    }
+  }
 />
 `;
 
@@ -404,6 +409,11 @@ exports[`EuiInMemoryTable with pagination, selection, sorting  and simple search
         "onSelectionChanged": [Function],
       }
     }
+    sorting={
+      Object {
+        "sort": undefined,
+      }
+    }
   />
 </div>
 `;
@@ -467,6 +477,11 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and a single recor
       "onSelectionChanged": [Function],
     }
   }
+  sorting={
+    Object {
+      "sort": undefined,
+    }
+  }
 />
 `;
 
@@ -515,6 +530,11 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and column rendere
       "onSelectionChanged": [Function],
     }
   }
+  sorting={
+    Object {
+      "sort": undefined,
+    }
+  }
 />
 `;
 
@@ -596,6 +616,11 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and configured sea
         "onSelectionChanged": [Function],
       }
     }
+    sorting={
+      Object {
+        "sort": undefined,
+      }
+    }
   />
 </div>
 `;
@@ -630,5 +655,10 @@ exports[`EuiInMemoryTable with sorting 1`] = `
   }
   noItemsMessage="No items found"
   onChange={[Function]}
+  sorting={
+    Object {
+      "sort": undefined,
+    }
+  }
 />
 `;
diff --git a/src/components/basic_table/basic_table.js b/src/components/basic_table/basic_table.js
@@ -368,18 +368,19 @@ export class EuiBasicTable extends Component {
       }
 
       // field data column
-      const sortDirection = this.resolveColumnSortDirection(column);
-      const onSort = this.resolveColumnOnSort(column);
-      const isSorted = !!sortDirection;
-      const isSortAscending = SortDirection.isAsc(sortDirection);
+      const sorting = {};
+      if (this.props.sorting && column.sortable) {
+        const sortDirection = this.resolveColumnSortDirection(column);
+        sorting.isSorted = !!sortDirection;
+        sorting.isSortAscending = sortDirection ? SortDirection.isAsc(sortDirection) : undefined;
+        sorting.onSort = this.resolveColumnOnSort(column);
+      }
       headers.push(
         <EuiTableHeaderCell
           key={`_data_h_${column.field}_${index}`}
           align={align}
-          isSorted={isSorted}
-          isSortAscending={isSortAscending}
-          onSort={onSort}
           width={column.width}
+          {...sorting}
         >
           {column.name}
         </EuiTableHeaderCell>
@@ -604,13 +605,15 @@ export class EuiBasicTable extends Component {
   }
 
   resolveColumnOnSort(column) {
-    if (column.sortable) {
-      if (!this.props.onChange) {
-        throw new Error(`BasicTable is configured to be sortable on column [${column.field}] but
+    const { sorting } = this.props;
+    if (!sorting || !column.sortable) {
+      return;
+    }
+    if (!this.props.onChange) {
+      throw new Error(`BasicTable is configured to be sortable on column [${column.field}] but
           [onChange] is not configured. This callback must be implemented to handle the sort requests`);
-      }
-      return () => this.onColumnSortChange(column);
     }
+    return () => this.onColumnSortChange(column);
   }
 
   resolveContentRenderer(column) {

diff --git a/src/components/basic_table/basic_table.test.js b/src/components/basic_table/basic_table.test.js
@@ -211,6 +211,32 @@ describe('EuiBasicTable', () => {
     expect(component).toMatchSnapshot();
   });
 
+  test('with sortable columns and sorting disabled', () => {
+
+    const props = {
+      ...requiredProps,
+      items: [
+        { id: '1', name: 'name1' },
+        { id: '2', name: 'name2' },
+        { id: '3', name: 'name3' }
+      ],
+      columns: [
+        {
+          field: 'name',
+          name: 'Name',
+          description: 'description',
+          sortable: true
+        }
+      ],
+      onChange: () => {}
+    };
+    const component = shallow(
+      <EuiBasicTable {...props} />
+    );
+
+    expect(component).toMatchSnapshot();
+  });
+
   test('with pagination and selection', () => {
 
     const props = {

diff --git a/src/components/basic_table/in_memory_table.js b/src/components/basic_table/in_memory_table.js
@@ -110,7 +110,7 @@ export class EuiInMemoryTable extends Component {
   constructor(props) {
     super(props);
 
-    const { search, pagination, sorting, columns } = props;
+    const { search, pagination, sorting } = props;
     const { pageIndex, pageSize, pageSizeOptions } = getInitialPagination(pagination);
     const { sortField, sortDirection } = getInitialSorting(sorting);
 
@@ -238,8 +238,8 @@ export class EuiInMemoryTable extends Component {
     // user, but can't be reproduced with client-side sort logic. So we allow the table to display
     // rows in the order in which they're initially loaded by providing an undefined sorting prop.
     // Once a user sorts a column, this will become a fully-defined sorting prop.
-    const sorting = !hasSorting || (!sortField && !sortDirection) ? undefined : {
-      sort: {
+    const sorting = !hasSorting ? undefined : {
+      sort: (!sortField && !sortDirection) ? undefined : {
         field: sortField,
         direction: sortDirection,
       },