Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiInMemoryTable] Fix onTableChange returning the sort field's name instead of field key #5588

Merged
merged 8 commits into from
Feb 2, 2022
Merged
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## [`main`](https://github.com/elastic/eui/tree/main)

No public interface changes since `46.2.0`.
**Bug fixes**

- Fixed `EuiInMemoryTable`'s `onTableChange` callback not returning the correct `sort.field` value on pagination ([#5588](https://github.com/elastic/eui/pull/5588))

## [`46.2.0`](https://github.com/elastic/eui/tree/v46.2.0)

Expand Down
27 changes: 22 additions & 5 deletions src/components/basic_table/in_memory_table.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1015,28 +1015,29 @@ describe('EuiInMemoryTable', () => {
};

const component = mount(<EuiInMemoryTable {...props} />);

expect(props.onTableChange).toHaveBeenCalledTimes(0);

// Pagination change
component
.find('EuiButtonEmpty[data-test-subj="pagination-button-1"]')
.simulate('click');
expect(props.onTableChange).toHaveBeenCalledTimes(1);
expect(props.onTableChange).toHaveBeenCalledWith({
expect(props.onTableChange).toHaveBeenLastCalledWith({
sort: {},
page: {
index: 1,
size: 2,
},
});

(props.onTableChange as jest.Mock).mockClear();
// Sorting change
component
.find(
'[data-test-subj*="tableHeaderCell_name_0"] [data-test-subj="tableHeaderSortButton"]'
)
.simulate('click');
expect(props.onTableChange).toHaveBeenCalledTimes(1);
expect(props.onTableChange).toHaveBeenCalledWith({
expect(props.onTableChange).toHaveBeenCalledTimes(2);
expect(props.onTableChange).toHaveBeenLastCalledWith({
sort: {
direction: SortDirection.ASC,
field: 'name',
Expand All @@ -1046,6 +1047,22 @@ describe('EuiInMemoryTable', () => {
size: 2,
},
});

// Sorted pagination change
component
.find('EuiButtonEmpty[data-test-subj="pagination-button-1"]')
.simulate('click');
expect(props.onTableChange).toHaveBeenCalledTimes(3);
expect(props.onTableChange).toHaveBeenLastCalledWith({
sort: {
direction: SortDirection.ASC,
field: 'name',
},
page: {
index: 1,
size: 2,
},
});
});
});

Expand Down
43 changes: 26 additions & 17 deletions src/components/basic_table/in_memory_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ const getInitialPagination = (pagination: Pagination | undefined) => {
function findColumnByProp<T>(
columns: Array<EuiBasicTableColumn<T>>,
prop: 'field' | 'name',
value: string
value: string | ReactNode
) {
for (let i = 0; i < columns.length; i++) {
const column = columns[i];
Expand All @@ -202,6 +202,19 @@ function findColumnByProp<T>(
}
}

function findColumnByFieldOrName<T>(
columns: Array<EuiBasicTableColumn<T>>,
value: string | ReactNode
) {
// The passed value can be a column's `field` or its `name`
// for backwards compatibility `field` must be checked first
let column = findColumnByProp(columns, 'field', value);
if (column == null) {
column = findColumnByProp(columns, 'name', value);
}
return column;
}

function getInitialSorting<T>(
columns: Array<EuiBasicTableColumn<T>>,
sorting: Sorting | undefined
Expand All @@ -218,12 +231,7 @@ function getInitialSorting<T>(
direction: sortDirection,
} = (sorting as SortingOptions).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 sortColumn = findColumnByFieldOrName(columns, sortable);

if (sortColumn == null) {
return {
Expand Down Expand Up @@ -401,16 +409,17 @@ export class EuiInMemoryTable<T> extends Component<
// 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 (
'field' in column &&
(column as EuiTableFieldDataColumnType<T>).field === sortName
) {
sortName = column.name as keyof T;
break;
// EuiBasicTable returns the column's `field` instead of `name` on sort
// and the column's `name` instead of `field` on pagination
if (sortName) {
const sortColumn = findColumnByFieldOrName(this.props.columns, sortName);
if (sortColumn) {
// Ensure sortName uses `name`
sortName = sortColumn.name as keyof T;

// Ensure reportedSortName uses `field` if it exists
const sortField = (sortColumn as EuiTableFieldDataColumnType<T>).field;
if (sortField) reportedSortName = sortField as keyof T;
}
}

Expand Down