Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- Fixed a focus bug in `EuiDataGrid` when clicking another cell header with an already-open cell header popover ([#5556](https://github.com/elastic/eui/pull/5556))
- Fixed `EuiDataGrid` to always focus back into the grid on pagination ([#5587](https://github.com/elastic/eui/pull/5587))
- Fixed `EuiDataGrid` and `EuiTable` pagination potentially rendering out view on narrow tables with many pages ([#5561](https://github.com/elastic/eui/pull/5561))
- Fixed `EuiInMemoryTable`'s `onTableChange` callback not returning the correct `sort.field` value on pagination ([#5588](https://github.com/elastic/eui/pull/5588))

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

Expand Down
1 change: 1 addition & 0 deletions src-docs/src/views/tables/in_memory/in_memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export const Table = () => {
columns={columns}
pagination={true}
sorting={sorting}
onTableChange={(values) => console.log(values.sort)}
/>
);
};
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
25 changes: 15 additions & 10 deletions src/components/basic_table/in_memory_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -401,16 +401,21 @@ 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 { columns } = this.props;
let sortColumn = findColumnByProp(columns, 'field', sortName as string);
if (sortColumn == null) {
sortColumn = findColumnByProp(columns, 'name', sortName as string);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this handy 'findColumnBy' logic up above in getInitialSorting:

// 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);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want to make this a function that's reused both places?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooo, you know it!! 16dc234

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