Skip to content

Commit

Permalink
Allow sorting on multiple columns in Discover (elastic#41918)
Browse files Browse the repository at this point in the history
This commit enables sorting on multiple columns in Discover and in saved search panels on dashboards. The UI is simple and should be familiar based on the way multi-sort works in other common applications like file explorers. Each sortable column has a sort icon indicating which way it is sorted (or unsorted, in the case of two arrows pointing both up and down). Sort priority is determined by which column was clicked most recently, with the most recent being the lowest priority.
  • Loading branch information
Bargs committed Aug 2, 2019
1 parent 27f1696 commit 91305bd
Show file tree
Hide file tree
Showing 12 changed files with 129 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,8 @@ function discoverController(
$scope.$watchCollection('state.sort', function (sort) {
if (!sort) return;

// get the current sort from {key: val} to ["key", "val"];
const currentSort = _.pairs($scope.searchSource.getField('sort')).pop();
// get the current sort from searchSource as array of arrays
const currentSort = getSort.array($scope.searchSource.getField('sort'), $scope.indexPattern);

// if the searchSource doesn't know, tell it so
if (!angular.equals(sort, currentSort)) $scope.fetch();
Expand Down Expand Up @@ -862,8 +862,8 @@ function discoverController(
.setField('filter', queryFilter.getFilters());
});

$scope.setSortOrder = function setSortOrder(columnName, direction) {
$scope.state.sort = [columnName, direction];
$scope.setSortOrder = function setSortOrder(sortPair) {
$scope.state.sort = sortPair;
};

// TODO: On array fields, negating does not negate the combination, rather all terms
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import ngMock from 'ng_mock';
import { getSort } from '../../lib/get_sort';
import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';

const defaultSort = { time: 'desc' };
const defaultSort = [{ time: 'desc' }];
let indexPattern;

describe('docTable', function () {
Expand All @@ -38,19 +38,19 @@ describe('docTable', function () {
expect(getSort).to.be.a(Function);
});

it('should return an object if passed a 2 item array', function () {
expect(getSort(['bytes', 'desc'], indexPattern)).to.eql({ bytes: 'desc' });
it('should return an array of objects if passed a 2 item array', function () {
expect(getSort(['bytes', 'desc'], indexPattern)).to.eql([{ bytes: 'desc' }]);

delete indexPattern.timeFieldName;
expect(getSort(['bytes', 'desc'], indexPattern)).to.eql({ bytes: 'desc' });
expect(getSort(['bytes', 'desc'], indexPattern)).to.eql([{ bytes: 'desc' }]);
});

it('should sort by the default when passed an unsortable field', function () {
expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql(defaultSort);
expect(getSort(['lol_nope', 'asc'], indexPattern)).to.eql(defaultSort);

delete indexPattern.timeFieldName;
expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql({ _score: 'desc' });
expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql([{ _score: 'desc' }]);
});

it('should sort in reverse chrono order otherwise on time based patterns', function () {
Expand All @@ -62,9 +62,9 @@ describe('docTable', function () {
it('should sort by score on non-time patterns', function () {
delete indexPattern.timeFieldName;

expect(getSort([], indexPattern)).to.eql({ _score: 'desc' });
expect(getSort(['foo'], indexPattern)).to.eql({ _score: 'desc' });
expect(getSort({ foo: 'bar' }, indexPattern)).to.eql({ _score: 'desc' });
expect(getSort([], indexPattern)).to.eql([{ _score: 'desc' }]);
expect(getSort(['foo'], indexPattern)).to.eql([{ _score: 'desc' }]);
expect(getSort({ foo: 'bar' }, indexPattern)).to.eql([{ _score: 'desc' }]);
});
});

Expand All @@ -73,8 +73,8 @@ describe('docTable', function () {
expect(getSort.array).to.be.a(Function);
});

it('should return an array for sortable fields', function () {
expect(getSort.array(['bytes', 'desc'], indexPattern)).to.eql([ 'bytes', 'desc' ]);
it('should return an array of arrays for sortable fields', function () {
expect(getSort.array(['bytes', 'desc'], indexPattern)).to.eql([[ 'bytes', 'desc' ]]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function getMockProps(props = {}) {
indexPattern: getMockIndexPattern(),
hideTimeColumn: false,
columns: ['first', 'middle', 'last'],
sortOrder: ['time', 'asc'] as SortOrder,
sortOrder: [['time', 'asc']] as SortOrder[],
isShortDots: true,
onRemoveColumn: jest.fn(),
onChangeSortOrder: jest.fn(),
Expand Down Expand Up @@ -89,7 +89,7 @@ describe('TableHeader with time column', () => {

test('time column is sortable with button, cycling sort direction', () => {
findTestSubject(wrapper, 'docTableHeaderFieldSort_time').simulate('click');
expect(props.onChangeSortOrder).toHaveBeenCalledWith('time', 'desc');
expect(props.onChangeSortOrder).toHaveBeenCalledWith([['time', 'desc']]);
});

test('time column is not removeable, no button displayed', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ interface Props {
hideTimeColumn: boolean;
indexPattern: IndexPatternEnhanced;
isShortDots: boolean;
onChangeSortOrder?: (name: string, direction: 'asc' | 'desc') => void;
onChangeSortOrder?: (sortOrder: SortOrder[]) => void;
onMoveColumn?: (name: string, index: number) => void;
onRemoveColumn?: (name: string) => void;
sortOrder: SortOrder;
sortOrder: SortOrder[];
}

export function TableHeader({
Expand All @@ -45,7 +45,6 @@ export function TableHeader({
sortOrder,
}: Props) {
const displayedColumns = getDisplayedColumns(columns, indexPattern, hideTimeColumn, isShortDots);
const [currColumnName, currDirection = 'asc'] = sortOrder;

return (
<tr data-test-subj="docTableHeader" className="kbnDocTableHeader">
Expand All @@ -55,7 +54,7 @@ export function TableHeader({
<TableHeaderColumn
key={col.name}
{...col}
sortDirection={col.name === currColumnName ? currDirection : ''}
sortOrder={sortOrder}
onMoveColumn={onMoveColumn}
onRemoveColumn={onRemoveColumn}
onChangeSortOrder={onChangeSortOrder}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { i18n } from '@kbn/i18n';
import { EuiToolTip } from '@elastic/eui';
// @ts-ignore
import { shortenDottedString } from '../../../../../common/utils/shorten_dotted_string';
import { SortOrder } from './helpers';

interface Props {
colLeftIdx: number; // idx of the column to the left, -1 if moving is not possible
Expand All @@ -30,12 +31,18 @@ interface Props {
isRemoveable: boolean;
isSortable: boolean;
name: string;
onChangeSortOrder?: (name: string, direction: 'asc' | 'desc') => void;
onChangeSortOrder?: (sortOrder: SortOrder[]) => void;
onMoveColumn?: (name: string, idx: number) => void;
onRemoveColumn?: (name: string) => void;
sortDirection: 'asc' | 'desc' | ''; // asc|desc -> field is sorted in this direction, else ''
sortOrder: SortOrder[];
}

const sortDirectionToIcon = {
desc: 'fa fa-sort-down',
asc: 'fa fa-sort-up',
'': 'fa fa-sort',
};

export function TableHeaderColumn({
colLeftIdx,
colRightIdx,
Expand All @@ -46,43 +53,78 @@ export function TableHeaderColumn({
onChangeSortOrder,
onMoveColumn,
onRemoveColumn,
sortDirection,
sortOrder,
}: Props) {
const btnSortIcon = sortDirection === 'desc' ? 'fa fa-sort-down' : 'fa fa-sort-up';
const [, sortDirection = ''] = sortOrder.find(sortPair => name === sortPair[0]) || [];
const currentSortWithoutColumn = sortOrder.filter(pair => pair[0] !== name);
const currentColumnSort = sortOrder.find(pair => pair[0] === name);
const currentColumnSortDirection = (currentColumnSort && currentColumnSort[1]) || '';

const btnSortIcon = sortDirectionToIcon[sortDirection];
const btnSortClassName =
sortDirection !== '' ? btnSortIcon : `kbnDocTableHeader__sortChange ${btnSortIcon}`;

const handleChangeSortOrder = () => {
if (!onChangeSortOrder) return;

// Cycle goes Unsorted -> Asc -> Desc -> Unsorted
if (currentColumnSort === undefined) {
onChangeSortOrder([...currentSortWithoutColumn, [name, 'asc']]);
} else if (currentColumnSortDirection === 'asc') {
onChangeSortOrder([...currentSortWithoutColumn, [name, 'desc']]);
} else if (currentColumnSortDirection === 'desc' && currentSortWithoutColumn.length === 0) {
// If we're at the end of the cycle and this is the only existing sort, we switch
// back to ascending sort instead of removing it.
onChangeSortOrder([[name, 'asc']]);
} else {
onChangeSortOrder(currentSortWithoutColumn);
}
};

const getSortButtonAriaLabel = () => {
const sortAscendingMessage = i18n.translate(
'kbn.docTable.tableHeader.sortByColumnAscendingAriaLabel',
{
defaultMessage: 'Sort {columnName} ascending',
values: { columnName: name },
}
);
const sortDescendingMessage = i18n.translate(
'kbn.docTable.tableHeader.sortByColumnDescendingAriaLabel',
{
defaultMessage: 'Sort {columnName} descending',
values: { columnName: name },
}
);
const stopSortingMessage = i18n.translate(
'kbn.docTable.tableHeader.sortByColumnUnsortedAriaLabel',
{
defaultMessage: 'Stop sorting on {columnName}',
values: { columnName: name },
}
);

if (currentColumnSort === undefined) {
return sortAscendingMessage;
} else if (sortDirection === 'asc') {
return sortDescendingMessage;
} else if (sortDirection === 'desc' && currentSortWithoutColumn.length === 0) {
return sortAscendingMessage;
} else {
return stopSortingMessage;
}
};

// action buttons displayed on the right side of the column name
const buttons = [
// Sort Button
{
active: isSortable && typeof onChangeSortOrder === 'function',
ariaLabel:
sortDirection === 'asc'
? i18n.translate('kbn.docTable.tableHeader.sortByColumnDescendingAriaLabel', {
defaultMessage: 'Sort {columnName} descending',
values: { columnName: name },
})
: i18n.translate('kbn.docTable.tableHeader.sortByColumnAscendingAriaLabel', {
defaultMessage: 'Sort {columnName} ascending',
values: { columnName: name },
}),
ariaLabel: getSortButtonAriaLabel(),
className: btnSortClassName,
onClick: () => {
/**
* cycle sorting direction
* asc -> desc, desc -> asc, default: asc
*/
if (typeof onChangeSortOrder === 'function') {
const newDirection = sortDirection === 'asc' ? 'desc' : 'asc';
onChangeSortOrder(name, newDirection);
}
},
onClick: handleChangeSortOrder,
testSubject: `docTableHeaderFieldSort_${name}`,
tooltip: i18n.translate('kbn.docTable.tableHeader.sortByColumnTooltip', {
defaultMessage: 'Sort by {columnName}',
values: { columnName: name },
}),
tooltip: getSortButtonAriaLabel(),
},
// Remove Button
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
filters="filters"
class="kbnDocTable__row"
on-add-column="onAddColumn"
on-change-sort-order="onChangeSortOrder"
on-remove-column="onRemoveColumn"
></tr>
</tbody>
Expand Down Expand Up @@ -98,7 +97,6 @@
ng-class="{'kbnDocTable__row--highlight': row['$$_isAnchor']}"
data-test-subj="docTableRow{{ row['$$_isAnchor'] ? ' docTableAnchorRow' : ''}}"
on-add-column="onAddColumn"
on-change-sort-order="onChangeSortOrder"
on-remove-column="onRemoveColumn"
></tr>
</tbody>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,42 +19,49 @@

import _ from 'lodash';

function isSortable(field, indexPattern) {
return (indexPattern.fields.byName[field] && indexPattern.fields.byName[field].sortable);
}

function createSortObject(sortPair, indexPattern) {

if (Array.isArray(sortPair) && sortPair.length === 2 && isSortable(sortPair[0], indexPattern)) {
const [ field, direction ] = sortPair;
return { [field]: direction };
}
else {
return undefined;
}
}

/**
* Take a sorting array and make it into an object
* @param {array} 2 item array [fieldToSort, directionToSort]
* @param {array} sort 2 item array [fieldToSort, directionToSort]
* @param {object} indexPattern used for determining default sort
* @returns {object} a sort object suitable for returning to elasticsearch
*/
export function getSort(sort, indexPattern, defaultSortOrder = 'desc') {
const sortObj = {};
let field;
let direction;

function isSortable(field) {
return (indexPattern.fields.byName[field] && indexPattern.fields.byName[field].sortable);
let sortObjects;
if (Array.isArray(sort)) {
if (sort.length > 0 && !Array.isArray(sort[0])) {
sort = [sort];
}
sortObjects = _.compact(sort.map((sortPair) => createSortObject(sortPair, indexPattern)));
}

if (Array.isArray(sort) && sort.length === 2 && isSortable(sort[0])) {
// At some point we need to refactor the sorting logic, this array sucks.
field = sort[0];
direction = sort[1];
} else if (indexPattern.timeFieldName && isSortable(indexPattern.timeFieldName)) {
field = indexPattern.timeFieldName;
direction = defaultSortOrder;
if (!_.isEmpty(sortObjects)) {
return sortObjects;
}

if (field) {
sortObj[field] = direction;
} else {
sortObj._score = 'desc';
else if (indexPattern.timeFieldName && isSortable(indexPattern.timeFieldName, indexPattern)) {
return [{ [indexPattern.timeFieldName]: defaultSortOrder }];
}
else {
return [{ _score: 'desc' }];
}



return sortObj;
}

getSort.array = function (sort, indexPattern, defaultSortOrder) {
return _(getSort(sort, indexPattern, defaultSortOrder)).pairs().pop();
return getSort(sort, indexPattern, defaultSortOrder).map((sortPair) => _(sortPair).pairs().pop());
};

Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ import { ISearchEmbeddable, SearchInput, SearchOutput } from './types';
interface SearchScope extends ng.IScope {
columns?: string[];
description?: string;
sort?: string[];
sort?: string[] | string[][];
searchSource?: SearchSource;
sharedItemTitle?: string;
inspectorAdapters?: Adapters;
setSortOrder?: (column: string, columnDirection: string) => void;
setSortOrder?: (sortPair: [string, string]) => void;
removeColumn?: (column: string) => void;
addColumn?: (column: string) => void;
moveColumn?: (column: string, index: number) => void;
Expand Down Expand Up @@ -201,8 +201,8 @@ export class SearchEmbeddable extends Embeddable<SearchInput, SearchOutput>

this.pushContainerStateParamsToScope(searchScope);

searchScope.setSortOrder = (columnName, direction) => {
searchScope.sort = [columnName, direction];
searchScope.setSortOrder = sortPair => {
searchScope.sort = sortPair;
this.updateInput({ sort: searchScope.sort });
};

Expand Down Expand Up @@ -263,6 +263,9 @@ export class SearchEmbeddable extends Embeddable<SearchInput, SearchOutput>
// been overridden in a dashboard.
searchScope.columns = this.input.columns || this.savedSearch.columns;
searchScope.sort = this.input.sort || this.savedSearch.sort;
if (searchScope.sort.length && !Array.isArray(searchScope.sort[0])) {
searchScope.sort = [searchScope.sort];
}
searchScope.sharedItemTitle = this.panelTitle;

if (
Expand Down
Loading

0 comments on commit 91305bd

Please sign in to comment.