From 3f53d546a0bee947af076a6a9c4b1c683e3c95b4 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 14 Jun 2019 11:58:05 -0600 Subject: [PATCH 1/9] Allow EuiInMemoryTable to sort computed columns --- .../in_memory/in_memory_custom_sorting.js | 21 ++-- .../in_memory_table.test.js.snap | 104 ++++++++---------- src/components/basic_table/basic_table.js | 30 ++++- src/components/basic_table/in_memory_table.js | 91 +++++++++------ .../basic_table/in_memory_table.test.js | 54 +++++---- 5 files changed, 175 insertions(+), 125 deletions(-) diff --git a/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js b/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js index 96eff3ad5c5..a87a97ead0c 100644 --- a/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js +++ b/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js @@ -2,12 +2,12 @@ import React from 'react'; import { EuiInMemoryTable } from '../../../../../src/components'; const data = [ - { animal: 'snail', weight: 25, humanFriendlyWeight: '25g' }, - { animal: 'peregrine falcon', weight: 900, humanFriendlyWeight: '0.9kg' }, - { animal: 'small dog', weight: 4500, humanFriendlyWeight: '4.5kg' }, - { animal: 'brown bear', weight: 180000, humanFriendlyWeight: '180kg' }, - { animal: 'elephant', weight: 5440000, humanFriendlyWeight: '5440kg' }, - { animal: 'giraffe', weight: 1180000, humanFriendlyWeight: '1180kg' }, + { animal: 'snail', weight: 25 }, + { animal: 'peregrine falcon', weight: 900 }, + { animal: 'small dog', weight: 4500 }, + { animal: 'brown bear', weight: 180000 }, + { animal: 'elephant', weight: 5440000 }, + { animal: 'giraffe', weight: 1180000 }, ]; export const Table = () => { @@ -18,15 +18,20 @@ export const Table = () => { sortable: true, }, { - field: 'humanFriendlyWeight', name: 'Weight', + render: ({ weight }) => `${weight / 1000}kg`, sortable: ({ weight }) => weight, }, + { + field: 'weight', + name: 'Weight (grams)', + sortable: true, + }, ]; const sorting = { sort: { - field: 'humanFriendlyWeight', + field: 'Weight', direction: 'asc', }, }; 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 index e825c9f5490..10645ae4fcf 100644 --- a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap +++ b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap @@ -56,7 +56,6 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -652,7 +651,6 @@ exports[`EuiInMemoryTable empty array 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -664,6 +662,49 @@ exports[`EuiInMemoryTable empty array 1`] = ` /> `; +exports[`EuiInMemoryTable sorting with field sorting 1`] = ` + +`; + exports[`EuiInMemoryTable with executeQueryOptions 1`] = ` `; - -exports[`EuiInMemoryTable with sorting 1`] = ` - -`; diff --git a/src/components/basic_table/basic_table.js b/src/components/basic_table/basic_table.js index 1d5b2dee2d3..8163110ebe5 100644 --- a/src/components/basic_table/basic_table.js +++ b/src/components/basic_table/basic_table.js @@ -112,7 +112,7 @@ export const FieldDataColumnTypeShape = { description: PropTypes.string, dataType: PropTypes.oneOf(DATA_TYPES), width: PropTypes.string, - sortable: PropTypes.bool, + sortable: PropTypes.oneOfType([PropTypes.bool, PropTypes.func]), align: PropTypes.oneOf([LEFT_ALIGNMENT, RIGHT_ALIGNMENT]), truncateText: PropTypes.bool, render: PropTypes.func, // ((value, record) => PropTypes.node (also see [services/value_renderer] for basic implementations) @@ -128,6 +128,7 @@ export const ComputedColumnType = PropTypes.shape({ render: PropTypes.func.isRequired, // (record) => PropTypes.node name: PropTypes.node, description: PropTypes.string, + sortable: PropTypes.func, width: PropTypes.string, truncateText: PropTypes.bool, }); @@ -326,7 +327,8 @@ export class EuiBasicTable extends Component { if ( currentCriteria && currentCriteria.sort && - currentCriteria.sort.field === column.field + (currentCriteria.sort.field === column.field || + currentCriteria.sort.field === column.name) ) { direction = SortDirection.reverse(currentCriteria.sort.direction); } @@ -340,7 +342,7 @@ export class EuiBasicTable extends Component { size: currentCriteria.page.size, }, sort: { - field: column.field, + field: column.field || column.name, direction, }, }; @@ -563,12 +565,25 @@ export class EuiBasicTable extends Component { // computed column if (!field) { + const sorting = {}; + // computed columns are only sortable if their `sortable` is a function + if (this.props.sorting && typeof sortable === 'function') { + const sortDirection = this.resolveColumnSortDirection(column); + sorting.isSorted = !!sortDirection; + sorting.isSortAscending = sortDirection + ? SortDirection.isAsc(sortDirection) + : undefined; + sorting.onSort = this.resolveColumnOnSort(column); + sorting.allowNeutralSort = this.props.sorting.allowNeutralSort; + } headers.push( + mobileOptions={mobileOptions} + data-test-subj={`tableHeaderCell_${name}_${index}`} + {...sorting}> {name} ); @@ -982,7 +997,10 @@ export class EuiBasicTable extends Component { if (!sorting || !sorting.sort || !column.sortable) { return; } - if (sorting.sort.field === column.field) { + if ( + sorting.sort.field === column.field || + sorting.sort.field === column.name + ) { return sorting.sort.direction; } }; @@ -994,7 +1012,7 @@ export class EuiBasicTable extends Component { } if (!this.props.onChange) { throw new Error(`BasicTable is configured to be sortable on column [${ - column.field + column.name }] but [onChange] is not configured. This callback must be implemented to handle the sort requests`); } diff --git a/src/components/basic_table/in_memory_table.js b/src/components/basic_table/in_memory_table.js index ea0d9477880..b3961e6d23f 100644 --- a/src/components/basic_table/in_memory_table.js +++ b/src/components/basic_table/in_memory_table.js @@ -132,18 +132,33 @@ const getInitialPagination = pagination => { }; }; -const getInitialSorting = sorting => { +function findColumnByProp(columns, prop, value) { + for (let i = 0; i < columns.length; i++) { + const column = columns[i]; + if (column[prop] === value) return column; + } +} + +const getInitialSorting = (columns, sorting) => { if (!sorting || !sorting.sort) { return { - sortField: undefined, + sortName: undefined, sortDirection: undefined, }; } - const { field: sortField, direction: sortDirection } = sorting.sort; + const { field: sortable, direction: sortDirection } = sorting.sort; + + // sortable could be a column's `field` or its `name` + // for backwards compatibility `field` must be checked first + let sortColumn = findColumnByProp(columns, 'field', sortable); + if (sortColumn == null) { + sortColumn = findColumnByProp(columns, 'name', sortable); + } + const sortName = sortColumn.name; return { - sortField, + sortName, sortDirection, }; }; @@ -169,13 +184,16 @@ export class EuiInMemoryTable extends Component { pageIndex: 0, }; } - const { sortField, sortDirection } = getInitialSorting(nextProps.sorting); + const { sortName, sortDirection } = getInitialSorting( + nextProps.columns, + nextProps.sorting + ); if ( - sortField !== prevState.prevProps.sortField || + sortName !== prevState.prevProps.sortName || sortDirection !== prevState.prevProps.sortDirection ) { return { - sortField, + sortName, sortDirection, }; } @@ -185,26 +203,26 @@ export class EuiInMemoryTable extends Component { constructor(props) { super(props); - const { search, pagination, sorting, allowNeutralSort } = props; + const { columns, search, pagination, sorting, allowNeutralSort } = props; const { pageIndex, pageSize, pageSizeOptions, hidePerPageOptions, } = getInitialPagination(pagination); - const { sortField, sortDirection } = getInitialSorting(sorting); + const { sortName, sortDirection } = getInitialSorting(columns, sorting); this.state = { prevProps: { items: props.items, - sortField, + sortName, sortDirection, }, query: getInitialQuery(search), pageIndex, pageSize, pageSizeOptions, - sortField, + sortName, sortDirection, allowNeutralSort: allowNeutralSort === false ? false : true, hidePerPageOptions, @@ -218,23 +236,33 @@ export class EuiInMemoryTable extends Component { const { index: pageIndex, size: pageSize } = page; - let { field: sortField, direction: sortDirection } = sort; + let { field: sortName, direction: sortDirection } = sort; + + // EuiBasicTable returns the column's `field` if it exists instead of `name`, + // map back to `name` if this is the case + for (let i = 0; i < this.props.columns.length; i++) { + const column = this.props.columns[i]; + if (column.field === sortName) { + sortName = column.name; + break; + } + } // Allow going back to 'neutral' sorting if ( this.state.allowNeutralSort && - this.state.sortField === sortField && + this.state.sortName === sortName && this.state.sortDirection === 'desc' && sortDirection === 'asc' ) { - sortField = ''; + sortName = ''; sortDirection = ''; } this.setState({ pageIndex, pageSize, - sortField, + sortName, sortDirection, }); }; @@ -289,11 +317,16 @@ export class EuiInMemoryTable extends Component { } getItemSorter() { - const { sortField, sortDirection } = this.state; + const { sortName, sortDirection } = this.state; const { columns } = this.props; - const sortColumn = columns.find(({ field }) => field === sortField); + const sortColumn = columns.find(({ name }) => name === sortName); + + if (sortColumn == null) { + // can't return a non-function so return a function that says everything is the same + return () => () => 0; + } let sortable; @@ -305,7 +338,10 @@ export class EuiInMemoryTable extends Component { return Comparators.value(sortable, Comparators.default(sortDirection)); } - return Comparators.property(sortField, Comparators.default(sortDirection)); + return Comparators.property( + sortColumn.field, + Comparators.default(sortDirection) + ); } getItems() { @@ -321,13 +357,13 @@ export class EuiInMemoryTable extends Component { }; } - const { query, sortField, pageIndex, pageSize } = this.state; + const { query, sortName, pageIndex, pageSize } = this.state; const matchingItems = query ? EuiSearchBar.Query.execute(query, items, executeQueryOptions) : items; - const sortedItems = sortField + const sortedItems = sortName ? matchingItems .slice(0) // avoid mutating the source array .sort(this.getItemSorter()) // sort, causes mutation @@ -377,7 +413,7 @@ export class EuiInMemoryTable extends Component { pageIndex, pageSize, pageSizeOptions, - sortField, + sortName, sortDirection, hidePerPageOptions, } = this.state; @@ -401,10 +437,10 @@ export class EuiInMemoryTable extends Component { ? undefined : { sort: - !sortField && !sortDirection + !sortName && !sortDirection ? undefined : { - field: sortField, + field: sortName, direction: sortDirection, }, allowNeutralSort: this.state.allowNeutralSort, @@ -412,20 +448,13 @@ export class EuiInMemoryTable extends Component { const searchBar = this.renderSearchBar(); - // EuiInMemoryTable's column type supports sortable as a function, but - // EuiBasicTable requires those functions to be cast to a boolean - const mappedColumns = columns.map(column => ({ - ...column, - sortable: !!column.sortable, - })); - const table = ( { expect(component).toMatchSnapshot(); }); - test('with sorting', () => { - 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, - }, - ], - sorting: true, - }; - const component = shallow(); + describe('sorting', () => { + test('with field sorting', () => { + 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, + }, + ], + sorting: true, + }; + const component = shallow(); - expect(component).toMatchSnapshot(); + expect(component).toMatchSnapshot(); + }); + + // test('with name sorting', () => { + // expect(true).toBe(false); + // }); + // + // test('verify field sorting trumps name sorting', () => { + // expect(true).toBe(false); + // }); + // + // test(`verify an invalid sort field doesn't blow everything up`, () => { + // expect(true).toBe(false); + // }); }); test('with initial sorting', () => { From 4870b5e24427ac245b7e215dbf094f2d470fe335 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 14 Jun 2019 12:17:56 -0600 Subject: [PATCH 2/9] New unit tests for computed table column sorting --- .../in_memory/in_memory_custom_sorting.js | 2 +- .../in_memory_table.test.js.snap | 134 +++++++++++----- src/components/basic_table/in_memory_table.js | 14 +- .../basic_table/in_memory_table.test.js | 145 ++++++++++++++++-- 4 files changed, 233 insertions(+), 62 deletions(-) diff --git a/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js b/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js index a87a97ead0c..00d42220ad8 100644 --- a/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js +++ b/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js @@ -31,7 +31,7 @@ export const Table = () => { const sorting = { sort: { - field: 'Weight', + field: 'Weightt', direction: 'asc', }, }; 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 index 10645ae4fcf..351d30866da 100644 --- a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap +++ b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap @@ -662,47 +662,99 @@ exports[`EuiInMemoryTable empty array 1`] = ` /> `; -exports[`EuiInMemoryTable sorting with field sorting 1`] = ` - +exports[`EuiInMemoryTable sorting verify field sorting precedes name sorting 1`] = ` +Array [ + + name3 + , + + 1 + , + + name2 + , + + 2 + , + + name1 + , + + 3 + , +] +`; + +exports[`EuiInMemoryTable sorting with field sorting (off by default) 1`] = ` +Array [ + + name3 + , + + name1 + , + + name2 + , +] +`; + +exports[`EuiInMemoryTable sorting with field sorting (on by default) 1`] = ` +Array [ + + name1 + , + + name2 + , + + name3 + , +] +`; + +exports[`EuiInMemoryTable sorting with name sorting 1`] = ` +Array [ + + name3 + , + + name2 + , + + name1 + , +] `; exports[`EuiInMemoryTable with executeQueryOptions 1`] = ` diff --git a/src/components/basic_table/in_memory_table.js b/src/components/basic_table/in_memory_table.js index b3961e6d23f..cf594586583 100644 --- a/src/components/basic_table/in_memory_table.js +++ b/src/components/basic_table/in_memory_table.js @@ -155,6 +155,14 @@ const getInitialSorting = (columns, sorting) => { if (sortColumn == null) { sortColumn = findColumnByProp(columns, 'name', sortable); } + + if (sortColumn == null) { + return { + sortName: undefined, + sortDirection: undefined, + }; + } + const sortName = sortColumn.name; return { @@ -328,11 +336,7 @@ export class EuiInMemoryTable extends Component { return () => () => 0; } - let sortable; - - if (sortColumn) { - sortable = sortColumn.sortable; - } + const sortable = sortColumn.sortable; if (typeof sortable === 'function') { return Comparators.value(sortable, Comparators.default(sortDirection)); diff --git a/src/components/basic_table/in_memory_table.test.js b/src/components/basic_table/in_memory_table.test.js index e49907c8c91..af2e89fbda7 100644 --- a/src/components/basic_table/in_memory_table.test.js +++ b/src/components/basic_table/in_memory_table.test.js @@ -245,13 +245,13 @@ describe('EuiInMemoryTable', () => { }); describe('sorting', () => { - test('with field sorting', () => { + test('with field sorting (off by default)', () => { const props = { ...requiredProps, items: [ + { id: '3', name: 'name3' }, { id: '1', name: 'name1' }, { id: '2', name: 'name2' }, - { id: '3', name: 'name3' }, ], columns: [ { @@ -263,22 +263,137 @@ describe('EuiInMemoryTable', () => { ], sorting: true, }; - const component = shallow(); + const component = mount(); - expect(component).toMatchSnapshot(); + expect( + component.find('tbody .euiTableCellContent__text') + ).toMatchSnapshot(); + }); + + test('with field sorting (on by default)', () => { + const props = { + ...requiredProps, + items: [ + { id: '3', name: 'name3' }, + { id: '1', name: 'name1' }, + { id: '2', name: 'name2' }, + ], + columns: [ + { + field: 'name', + name: 'Name', + description: 'description', + sortable: true, + }, + ], + sorting: { + sort: { + field: 'name', + direction: 'asc', + }, + }, + }; + const component = mount(); + + expect( + component.find('tbody .euiTableCellContent__text') + ).toMatchSnapshot(); + }); + + test('with name sorting', () => { + const props = { + ...requiredProps, + items: [ + { id: '3', name: 'name3' }, + { id: '1', name: 'name1' }, + { id: '2', name: 'name2' }, + ], + columns: [ + { + field: 'name', + name: 'Name', + description: 'description', + sortable: true, + }, + ], + sorting: { + sort: { + field: 'Name', + direction: 'desc', + }, + }, + }; + const component = mount(); + + expect( + component.find('tbody .euiTableCellContent__text') + ).toMatchSnapshot(); }); - // test('with name sorting', () => { - // expect(true).toBe(false); - // }); - // - // test('verify field sorting trumps name sorting', () => { - // expect(true).toBe(false); - // }); - // - // test(`verify an invalid sort field doesn't blow everything up`, () => { - // expect(true).toBe(false); - // }); + test('verify field sorting precedes name sorting', () => { + const props = { + ...requiredProps, + items: [ + { id: '1', name: 'name3' }, + { id: '3', name: 'name1' }, + { id: '2', name: 'name2' }, + ], + columns: [ + { + field: 'name', + name: 'Column 1', + description: 'description', + sortable: true, + }, + { + field: 'id', + name: 'name', + description: 'description', + sortable: true, + }, + ], + sorting: { + sort: { + field: 'name', + direction: 'desc', + }, + }, + }; + const component = mount(); + + // name TDs should be sorted desc, id TDs should be asc, + expect( + component.find('tbody .euiTableCellContent__text') + ).toMatchSnapshot(); + }); + + test(`verify an invalid sort field doesn't blow everything up`, () => { + const props = { + ...requiredProps, + items: [ + { id: '3', name: 'name3' }, + { id: '1', name: 'name1' }, + { id: '2', name: 'name2' }, + ], + columns: [ + { + field: 'name', + name: 'Name', + description: 'description', + sortable: true, + }, + ], + sorting: { + sort: { + field: 'something_nonexistant', + direction: 'asc', + }, + }, + }; + expect(() => { + mount(); + }).not.toThrow(); + }); }); test('with initial sorting', () => { From c1cb9fb0aa5d98d65dcd231ae27b4a193ce4fb29 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 14 Jun 2019 12:29:45 -0600 Subject: [PATCH 3/9] Updated props docs for tables' sortable prop --- src-docs/src/views/tables/basic/props_info.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src-docs/src/views/tables/basic/props_info.js b/src-docs/src/views/tables/basic/props_info.js index cf43a9a0301..f10a61b0b40 100644 --- a/src-docs/src/views/tables/basic/props_info.js +++ b/src-docs/src/views/tables/basic/props_info.js @@ -237,10 +237,11 @@ export const propsInfo = { type: { name: 'string (e.g. "30%", "100px", etc..)' }, }, sortable: { - description: 'Defines whether the user can sort on this column', + description: + 'Defines whether the user can sort on this column. If a function is provided, this function returns the value to sort against.', required: false, defaultValue: { value: 'false' }, - type: { name: 'boolean' }, + type: { name: '"boolean" | "function"' }, }, align: { description: 'Defines the horizontal alignment of the column', @@ -303,6 +304,12 @@ export const propsInfo = { required: false, type: { name: 'string (e.g. "30%", "100px", etc..)' }, }, + sortable: { + description: + 'If provided, allows this column to be sorted on. Must return the value to sort against.', + required: false, + type: { name: 'function' }, + }, truncateText: { description: "Indicates whether this column should truncate its content when it doesn't fit", From b9d8b7fdc5d15ac39925cd645813738c63e07247 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 14 Jun 2019 12:31:00 -0600 Subject: [PATCH 4/9] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b15cf081cc..bf2085746b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Fixed `EuiFlyout` scrolling in Safari ([#2033](https://github.com/elastic/eui/pull/2033)) - Fixed `EuiCallOut` header icon alignment ([#2006](https://github.com/elastic/eui/pull/2006)) - Fixed `EuiInMemoryTable` sort value persistence through lifecycle updates ([#2035](https://github.com/elastic/eui/pull/2035)) +- Fixed `EuiInMemoryTable` to allow sorting on computed columns ([#2044](https://github.com/elastic/eui/pull/2044)) **Breaking changes** From 43194c0241b2ddf553a8a9b15832e35c635cd94f Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 17 Jun 2019 09:08:07 -0600 Subject: [PATCH 5/9] Update src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js Co-Authored-By: Rory Hunter --- src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js b/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js index 00d42220ad8..a87a97ead0c 100644 --- a/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js +++ b/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js @@ -31,7 +31,7 @@ export const Table = () => { const sorting = { sort: { - field: 'Weightt', + field: 'Weight', direction: 'asc', }, }; From aefb8717115e677343a31ca17698adea5914432b Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 17 Jun 2019 09:25:52 -0600 Subject: [PATCH 6/9] PR feedback --- .../in_memory_table.test.js.snap | 95 ------------------- src/components/basic_table/in_memory_table.js | 4 +- .../basic_table/in_memory_table.test.js | 24 +++-- 3 files changed, 19 insertions(+), 104 deletions(-) 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 index 351d30866da..bf8c0f12ace 100644 --- a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap +++ b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap @@ -662,101 +662,6 @@ exports[`EuiInMemoryTable empty array 1`] = ` /> `; -exports[`EuiInMemoryTable sorting verify field sorting precedes name sorting 1`] = ` -Array [ - - name3 - , - - 1 - , - - name2 - , - - 2 - , - - name1 - , - - 3 - , -] -`; - -exports[`EuiInMemoryTable sorting with field sorting (off by default) 1`] = ` -Array [ - - name3 - , - - name1 - , - - name2 - , -] -`; - -exports[`EuiInMemoryTable sorting with field sorting (on by default) 1`] = ` -Array [ - - name1 - , - - name2 - , - - name3 - , -] -`; - -exports[`EuiInMemoryTable sorting with name sorting 1`] = ` -Array [ - - name3 - , - - name2 - , - - name1 - , -] -`; - exports[`EuiInMemoryTable with executeQueryOptions 1`] = ` { function findColumnByProp(columns, prop, value) { for (let i = 0; i < columns.length; i++) { const column = columns[i]; - if (column[prop] === value) return column; + if (column[prop] === value) { + return column; + } } } diff --git a/src/components/basic_table/in_memory_table.test.js b/src/components/basic_table/in_memory_table.test.js index af2e89fbda7..45337622cb8 100644 --- a/src/components/basic_table/in_memory_table.test.js +++ b/src/components/basic_table/in_memory_table.test.js @@ -266,8 +266,10 @@ describe('EuiInMemoryTable', () => { const component = mount(); expect( - component.find('tbody .euiTableCellContent__text') - ).toMatchSnapshot(); + component + .find('tbody .euiTableCellContent__text') + .map(cell => cell.text()) + ).toEqual(['name3', 'name1', 'name2']); }); test('with field sorting (on by default)', () => { @@ -296,8 +298,10 @@ describe('EuiInMemoryTable', () => { const component = mount(); expect( - component.find('tbody .euiTableCellContent__text') - ).toMatchSnapshot(); + component + .find('tbody .euiTableCellContent__text') + .map(cell => cell.text()) + ).toEqual(['name1', 'name2', 'name3']); }); test('with name sorting', () => { @@ -326,8 +330,10 @@ describe('EuiInMemoryTable', () => { const component = mount(); expect( - component.find('tbody .euiTableCellContent__text') - ).toMatchSnapshot(); + component + .find('tbody .euiTableCellContent__text') + .map(cell => cell.text()) + ).toEqual(['name3', 'name2', 'name1']); }); test('verify field sorting precedes name sorting', () => { @@ -363,8 +369,10 @@ describe('EuiInMemoryTable', () => { // name TDs should be sorted desc, id TDs should be asc, expect( - component.find('tbody .euiTableCellContent__text') - ).toMatchSnapshot(); + component + .find('tbody .euiTableCellContent__text') + .map(cell => cell.text()) + ).toEqual(['name3', '1', 'name2', '2', 'name1', '3']); }); test(`verify an invalid sort field doesn't blow everything up`, () => { From adf91356ef7ea4677d1667c0b90e8d949b101ac2 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 17 Jun 2019 10:11:57 -0600 Subject: [PATCH 7/9] make CI happy --- src/components/basic_table/in_memory_table.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/basic_table/in_memory_table.test.js b/src/components/basic_table/in_memory_table.test.js index 45337622cb8..5ea844886e8 100644 --- a/src/components/basic_table/in_memory_table.test.js +++ b/src/components/basic_table/in_memory_table.test.js @@ -375,7 +375,7 @@ describe('EuiInMemoryTable', () => { ).toEqual(['name3', '1', 'name2', '2', 'name1', '3']); }); - test(`verify an invalid sort field doesn't blow everything up`, () => { + test('verify an invalid sort field does not blow everything up', () => { const props = { ...requiredProps, items: [ From f3225351753231d9e6335bfdc95cddc61c8c08a8 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 17 Jun 2019 11:04:24 -0600 Subject: [PATCH 8/9] Fix bug in reporting table sort status to parent --- src/components/basic_table/in_memory_table.js | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/components/basic_table/in_memory_table.js b/src/components/basic_table/in_memory_table.js index 7173514d685..4fc5fab8a98 100644 --- a/src/components/basic_table/in_memory_table.js +++ b/src/components/basic_table/in_memory_table.js @@ -240,24 +240,10 @@ export class EuiInMemoryTable extends Component { } onTableChange = ({ page = {}, sort = {} }) => { - if (this.props.onTableChange) { - this.props.onTableChange({ page, sort }); - } - const { index: pageIndex, size: pageSize } = page; let { field: sortName, direction: sortDirection } = sort; - // EuiBasicTable returns the column's `field` if it exists instead of `name`, - // map back to `name` if this is the case - for (let i = 0; i < this.props.columns.length; i++) { - const column = this.props.columns[i]; - if (column.field === sortName) { - sortName = column.name; - break; - } - } - // Allow going back to 'neutral' sorting if ( this.state.allowNeutralSort && @@ -269,6 +255,26 @@ export class EuiInMemoryTable extends Component { sortDirection = ''; } + if (this.props.onTableChange) { + this.props.onTableChange({ + page, + sort: { + field: sortName, + direction: sortDirection, + }, + }); + } + + // EuiBasicTable returns the column's `field` if it exists instead of `name`, + // map back to `name` if this is the case + for (let i = 0; i < this.props.columns.length; i++) { + const column = this.props.columns[i]; + if (column.field === sortName) { + sortName = column.name; + break; + } + } + this.setState({ pageIndex, pageSize, From 6cf2d6d20a3adb327dc50a5ac9d4a4f9f71485d9 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 17 Jun 2019 11:39:46 -0600 Subject: [PATCH 9/9] Fix column sort bug --- src/components/basic_table/in_memory_table.js | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/components/basic_table/in_memory_table.js b/src/components/basic_table/in_memory_table.js index 4fc5fab8a98..0805d7a44e9 100644 --- a/src/components/basic_table/in_memory_table.js +++ b/src/components/basic_table/in_memory_table.js @@ -244,6 +244,20 @@ export class EuiInMemoryTable extends Component { let { field: sortName, direction: sortDirection } = sort; + // To keep backwards compatibility reportedSortName needs to be tracked separately + // from sortName; sortName gets stored internally while reportedSortName is sent to the callback + let reportedSortName = sortName; + + // EuiBasicTable returns the column's `field` if it exists instead of `name`, + // map back to `name` if this is the case + for (let i = 0; i < this.props.columns.length; i++) { + const column = this.props.columns[i]; + if (column.field === sortName) { + sortName = column.name; + break; + } + } + // Allow going back to 'neutral' sorting if ( this.state.allowNeutralSort && @@ -252,6 +266,7 @@ export class EuiInMemoryTable extends Component { sortDirection === 'asc' ) { sortName = ''; + reportedSortName = ''; sortDirection = ''; } @@ -259,22 +274,12 @@ export class EuiInMemoryTable extends Component { this.props.onTableChange({ page, sort: { - field: sortName, + field: reportedSortName, direction: sortDirection, }, }); } - // EuiBasicTable returns the column's `field` if it exists instead of `name`, - // map back to `name` if this is the case - for (let i = 0; i < this.props.columns.length; i++) { - const column = this.props.columns[i]; - if (column.field === sortName) { - sortName = column.name; - break; - } - } - this.setState({ pageIndex, pageSize,