diff --git a/CHANGELOG.md b/CHANGELOG.md index 975a01d8242..d21970f5fe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,12 @@ ## [`master`](https://github.com/elastic/eui/tree/master) - Changed `EuiNavDrawerFlyout` title from `h5` to `div` ([#2040](https://github.com/elastic/eui/pull/2040)) + +**Bug fixes** + - Fixed proptype for `EuiCopy`'s `children` ([#2048](https://github.com/elastic/eui/pull/2048)) +- Fixed `EuiInMemoryTable` to allow sorting on computed columns ([#2044](https://github.com/elastic/eui/pull/2044)) +- Fixed `EuiInMemoryTable` to allow sorting on computed columns ([#2044](https://github.com/elastic/eui/pull/2044)) ## [`12.0.0`](https://github.com/elastic/eui/tree/v12.0.0) 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", 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..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 @@ -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, }, ] } @@ -674,7 +672,6 @@ exports[`EuiInMemoryTable with executeQueryOptions 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -725,7 +722,7 @@ exports[`EuiInMemoryTable with initial sorting 1`] = ` "allowNeutralSort": true, "sort": Object { "direction": "desc", - "field": "name", + "field": "Name", }, } } @@ -742,7 +739,6 @@ exports[`EuiInMemoryTable with items 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -779,7 +775,6 @@ exports[`EuiInMemoryTable with items and expanded item 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -824,7 +819,6 @@ exports[`EuiInMemoryTable with items and message - expecting to show the items 1 "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -861,7 +855,6 @@ exports[`EuiInMemoryTable with message 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -883,7 +876,6 @@ exports[`EuiInMemoryTable with message and loading 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -906,7 +898,6 @@ exports[`EuiInMemoryTable with pagination 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -952,7 +943,6 @@ exports[`EuiInMemoryTable with pagination and default page size and index 1`] = "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -994,7 +984,6 @@ exports[`EuiInMemoryTable with pagination and selection 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -1050,7 +1039,6 @@ exports[`EuiInMemoryTable with pagination, default page size and error 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -1093,7 +1081,6 @@ exports[`EuiInMemoryTable with pagination, hiding the per page options 1`] = ` "description": "description", "field": "name", "name": "Name", - "sortable": false, }, ] } @@ -1224,7 +1211,6 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and simple search }, ], "name": "Actions", - "sortable": false, }, ] } @@ -1299,7 +1285,6 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and a single recor }, ], "name": "Actions", - "sortable": false, }, ] } @@ -1459,7 +1444,6 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and configured sea }, ], "name": "Actions", - "sortable": false, }, ] } @@ -1503,46 +1487,3 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and configured sea /> `; - -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..0805d7a44e9 100644 --- a/src/components/basic_table/in_memory_table.js +++ b/src/components/basic_table/in_memory_table.js @@ -132,18 +132,43 @@ 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); + } + + if (sortColumn == null) { + return { + sortName: undefined, + sortDirection: undefined, + }; + } + + const sortName = sortColumn.name; return { - sortField, + sortName, sortDirection, }; }; @@ -169,13 +194,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 +213,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, @@ -212,29 +240,50 @@ 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: sortField, direction: sortDirection } = sort; + 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 && - this.state.sortField === sortField && + this.state.sortName === sortName && this.state.sortDirection === 'desc' && sortDirection === 'asc' ) { - sortField = ''; + sortName = ''; + reportedSortName = ''; sortDirection = ''; } + if (this.props.onTableChange) { + this.props.onTableChange({ + page, + sort: { + field: reportedSortName, + direction: sortDirection, + }, + }); + } + this.setState({ pageIndex, pageSize, - sortField, + sortName, sortDirection, }); }; @@ -289,23 +338,27 @@ 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); - - let sortable; + const sortColumn = columns.find(({ name }) => name === sortName); - if (sortColumn) { - sortable = sortColumn.sortable; + if (sortColumn == null) { + // can't return a non-function so return a function that says everything is the same + return () => () => 0; } + const sortable = sortColumn.sortable; + if (typeof sortable === 'function') { 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 +374,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 +430,7 @@ export class EuiInMemoryTable extends Component { pageIndex, pageSize, pageSizeOptions, - sortField, + sortName, sortDirection, hidePerPageOptions, } = this.state; @@ -401,10 +454,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 +465,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, + describe('sorting', () => { + test('with field sorting (off 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: true, + }; + const component = mount(); + + expect( + component + .find('tbody .euiTableCellContent__text') + .map(cell => cell.text()) + ).toEqual(['name3', 'name1', 'name2']); + }); + + 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', + }, }, - ], - sorting: true, - }; - const component = shallow(); + }; + const component = mount(); - expect(component).toMatchSnapshot(); + expect( + component + .find('tbody .euiTableCellContent__text') + .map(cell => cell.text()) + ).toEqual(['name1', 'name2', 'name3']); + }); + + 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') + .map(cell => cell.text()) + ).toEqual(['name3', 'name2', 'name1']); + }); + + 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') + .map(cell => cell.text()) + ).toEqual(['name3', '1', 'name2', '2', 'name1', '3']); + }); + + test('verify an invalid sort field does not 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', () => {