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

Prevent toggling loading on tables from re-mounting visible rows #2754

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
19 changes: 18 additions & 1 deletion src-docs/src/views/tables/basic/basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
EuiBasicTable,
EuiLink,
EuiHealth,
EuiPopover,
} from '../../../../../src/components';

/*
Expand All @@ -32,7 +33,22 @@ Example country object:

const store = createDataStore();

function CountryColumn({ country }) {
const [isOpen, setIsOpen] = React.useState(false);

return (
<EuiPopover
isOpen={isOpen}
closePopover={() => setIsOpen(false)}
button={<span onClick={() => setIsOpen(!isOpen)}>{country}</span>}>
Country: {country}
</EuiPopover>
);
}

export const Table = () => {
const [isLoading, setIsLoading] = React.useState(false);
window.setIsLoading = setIsLoading;
const columns = [
{
field: 'firstName',
Expand Down Expand Up @@ -82,7 +98,7 @@ export const Table = () => {
name: 'Nationality',
render: countryCode => {
const country = store.getCountry(countryCode);
return `${country.flag} ${country.name}`;
return <CountryColumn country={`${country.flag} ${country.name}`} />;
},
},
{
Expand Down Expand Up @@ -120,6 +136,7 @@ export const Table = () => {

return (
<EuiBasicTable
loading={isLoading}
items={items}
columns={columns}
rowProps={getRowProps}
Expand Down
76 changes: 57 additions & 19 deletions src/components/basic_table/__snapshots__/basic_table.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ exports[`EuiBasicTable cellProps renders cells with custom props from a callback
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelected={false}
>
Expand Down Expand Up @@ -175,7 +177,9 @@ exports[`EuiBasicTable cellProps renders rows with custom props from an object 1
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelected={false}
>
Expand Down Expand Up @@ -530,7 +534,9 @@ exports[`EuiBasicTable footers do not render without a column footer definition
Age
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelected={false}
>
Expand Down Expand Up @@ -769,7 +775,9 @@ exports[`EuiBasicTable footers render with pagination, selection, sorting, and f
Age
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelectable={true}
isSelected={false}
Expand Down Expand Up @@ -1031,7 +1039,9 @@ exports[`EuiBasicTable itemIdToExpandedRowMap renders an expanded row 1`] = `
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
aria-owns="row_1_expansion"
isSelected={false}
Expand Down Expand Up @@ -1157,7 +1167,9 @@ exports[`EuiBasicTable rowProps renders rows with custom props from a callback 1
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiKeyboardAccessible>
<EuiTableRow
className="customRowClass"
Expand Down Expand Up @@ -1284,7 +1296,9 @@ exports[`EuiBasicTable rowProps renders rows with custom props from an object 1`
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiKeyboardAccessible>
<EuiTableRow
className="customClass"
Expand Down Expand Up @@ -1411,7 +1425,9 @@ exports[`EuiBasicTable with pagination - 2nd page 1`] = `
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelected={false}
>
Expand Down Expand Up @@ -1517,7 +1533,9 @@ exports[`EuiBasicTable with pagination 1`] = `
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelected={false}
>
Expand Down Expand Up @@ -1733,7 +1751,9 @@ exports[`EuiBasicTable with pagination and selection 1`] = `
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelectable={true}
isSelected={false}
Expand Down Expand Up @@ -1889,7 +1909,9 @@ exports[`EuiBasicTable with pagination, hiding the per page options 1`] = `
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelected={false}
>
Expand Down Expand Up @@ -2048,7 +2070,9 @@ exports[`EuiBasicTable with pagination, selection and sorting 1`] = `
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelectable={true}
isSelected={false}
Expand Down Expand Up @@ -2245,7 +2269,9 @@ exports[`EuiBasicTable with pagination, selection, sorting and a single record a
Actions
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
hasActions={true}
isSelectable={true}
Expand Down Expand Up @@ -2523,7 +2549,9 @@ exports[`EuiBasicTable with pagination, selection, sorting and column dataType 1
Count
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelectable={true}
isSelected={false}
Expand Down Expand Up @@ -2714,7 +2742,9 @@ exports[`EuiBasicTable with pagination, selection, sorting and column renderer 1
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelectable={true}
isSelected={false}
Expand Down Expand Up @@ -2911,7 +2941,9 @@ exports[`EuiBasicTable with pagination, selection, sorting and multiple record a
Actions
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
hasActions={true}
isSelectable={true}
Expand Down Expand Up @@ -3207,7 +3239,9 @@ exports[`EuiBasicTable with pagination, selection, sorting, column renderer and
Count
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelectable={true}
isSelected={false}
Expand Down Expand Up @@ -3363,7 +3397,9 @@ exports[`EuiBasicTable with sortable columns and sorting disabled 1`] = `
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelected={false}
>
Expand Down Expand Up @@ -3492,7 +3528,9 @@ exports[`EuiBasicTable with sorting 1`] = `
Name
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<EuiTableRow
isSelected={false}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,9 @@ exports[`EuiInMemoryTable behavior pagination 1`] = `
</tr>
</thead>
</EuiTableHeader>
<EuiTableBody>
<EuiTableBody
bodyRef={[Function]}
>
<tbody>
<EuiTableRow
isSelected={false}
Expand Down

This file was deleted.

71 changes: 66 additions & 5 deletions src/components/basic_table/basic_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import { ExpandedItemActions } from './expanded_item_actions';

import { Pagination, PaginationBar } from './pagination_bar';
import { EuiIcon } from '../icon';
import { LoadingTableBody } from './loading_table_body';
import { EuiKeyboardAccessible, EuiScreenReaderOnly } from '../accessibility';
import { EuiI18n } from '../i18n';
import { EuiDelayRender } from '../delay_render';
Expand Down Expand Up @@ -258,10 +257,75 @@ export class EuiBasicTable<T = any> extends Component<
return null;
}

// used for moving in & out of `loading` state
private cleanups: Array<() => void> = [];
private tbody: HTMLTableSectionElement | null = null;

state = {
selection: [],
};

componentDidMount() {
if (this.props.loading && this.tbody) this.addLoadingListeners(this.tbody);
}

componentDidUpdate(prevProps: EuiBasicTableProps<T>) {
if (prevProps.loading !== this.props.loading) {
if (this.props.loading && this.tbody) {
this.addLoadingListeners(this.tbody);
} else {
this.removeLoadingListeners();
}
}
}

componentWillUnmount() {
this.removeLoadingListeners();
}

private setTbody = (tbody: HTMLTableSectionElement | null) => {
// remove listeners from an existing element
this.removeLoadingListeners();

// update the ref
this.tbody = tbody;

// if loading, add listeners
if (this.props.loading === true && tbody) {
this.addLoadingListeners(tbody);
}
};

private addLoadingListeners = (tbody: HTMLTableSectionElement) => {
const listener = (event: Event) => {
event.stopPropagation();
event.preventDefault();
};
[
'mousedown',
'mouseup',
'mouseover',
'mouseout',
'mouseenter',
'mouseleave',
'click',
'dblclick',
'keydown',
'keyup',
'keypress',
].forEach(event => {
tbody.addEventListener(event, listener, true);
this.cleanups.push(() => {
tbody.removeEventListener(event, listener, true);
});
});
};

private removeLoadingListeners = () => {
this.cleanups.forEach(cleanup => cleanup());
this.cleanups.length = 0;
};

buildCriteria(props: EuiBasicTableProps<T>): Criteria<T> {
const criteria: Criteria<T> = {};
if (hasPagination(props)) {
Expand Down Expand Up @@ -702,10 +766,7 @@ export class EuiBasicTable<T = any> extends Component<
: index;
return this.renderItemRow(item, tableItemIndex);
});
if (this.props.loading) {
return <LoadingTableBody>{rows}</LoadingTableBody>;
}
return <EuiTableBody>{rows}</EuiTableBody>;
return <EuiTableBody bodyRef={this.setTbody}>{rows}</EuiTableBody>;
}

renderErrorBody(error: string) {
Expand Down
Loading