From ea122a5ad4f4cea5437a25bbf26bb94e9738aad5 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 13 Jan 2020 11:48:59 -0700 Subject: [PATCH 1/4] Removed EuiLoadingTableBody & moved its logic into EuiBasicTable to prevent re-rendering the whole table when loading is set to true --- .../__snapshots__/basic_table.test.tsx.snap | 76 ++++++++++++++----- .../in_memory_table.test.tsx.snap | 4 +- .../loading_table_body.test.tsx.snap | 17 ----- src/components/basic_table/basic_table.tsx | 71 +++++++++++++++-- .../basic_table/loading_table_body.test.tsx | 20 ----- .../basic_table/loading_table_body.tsx | 53 ------------- 6 files changed, 126 insertions(+), 115 deletions(-) delete mode 100644 src/components/basic_table/__snapshots__/loading_table_body.test.tsx.snap delete mode 100644 src/components/basic_table/loading_table_body.test.tsx delete mode 100644 src/components/basic_table/loading_table_body.tsx diff --git a/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap b/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap index 1a6e6bfebf7..3f16814e351 100644 --- a/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap +++ b/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap @@ -54,7 +54,9 @@ exports[`EuiBasicTable cellProps renders cells with custom props from a callback Name - + @@ -175,7 +177,9 @@ exports[`EuiBasicTable cellProps renders rows with custom props from an object 1 Name - + @@ -530,7 +534,9 @@ exports[`EuiBasicTable footers do not render without a column footer definition Age - + @@ -769,7 +775,9 @@ exports[`EuiBasicTable footers render with pagination, selection, sorting, and f Age - + - + - + - + - + @@ -1517,7 +1533,9 @@ exports[`EuiBasicTable with pagination 1`] = ` Name - + @@ -1733,7 +1751,9 @@ exports[`EuiBasicTable with pagination and selection 1`] = ` Name - + - + @@ -2048,7 +2070,9 @@ exports[`EuiBasicTable with pagination, selection and sorting 1`] = ` Name - + - + - + - + - + - + - + @@ -3492,7 +3528,9 @@ exports[`EuiBasicTable with sorting 1`] = ` Name - + diff --git a/src/components/basic_table/__snapshots__/in_memory_table.test.tsx.snap b/src/components/basic_table/__snapshots__/in_memory_table.test.tsx.snap index 8e7e8fdbdef..34dd0470f1d 100644 --- a/src/components/basic_table/__snapshots__/in_memory_table.test.tsx.snap +++ b/src/components/basic_table/__snapshots__/in_memory_table.test.tsx.snap @@ -177,7 +177,9 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` - + - - - - - - -`; diff --git a/src/components/basic_table/basic_table.tsx b/src/components/basic_table/basic_table.tsx index 078bf9a74f2..cab3317f1cb 100644 --- a/src/components/basic_table/basic_table.tsx +++ b/src/components/basic_table/basic_table.tsx @@ -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'; @@ -258,10 +257,75 @@ export class EuiBasicTable 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) { + 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): Criteria { const criteria: Criteria = {}; if (hasPagination(props)) { @@ -702,10 +766,7 @@ export class EuiBasicTable extends Component< : index; return this.renderItemRow(item, tableItemIndex); }); - if (this.props.loading) { - return {rows}; - } - return {rows}; + return {rows}; } renderErrorBody(error: string) { diff --git a/src/components/basic_table/loading_table_body.test.tsx b/src/components/basic_table/loading_table_body.test.tsx deleted file mode 100644 index 182dba0e23f..00000000000 --- a/src/components/basic_table/loading_table_body.test.tsx +++ /dev/null @@ -1,20 +0,0 @@ -import React from 'react'; -import { mount } from 'enzyme'; -import { requiredProps } from '../../test'; - -import { LoadingTableBody } from './loading_table_body'; - -describe('LoadingTableBody', () => { - test('render', () => { - const props = { - ...requiredProps, - }; - const component = mount( - - -
- ); - - expect(component).toMatchSnapshot(); - }); -}); diff --git a/src/components/basic_table/loading_table_body.tsx b/src/components/basic_table/loading_table_body.tsx deleted file mode 100644 index b720c4f7bc6..00000000000 --- a/src/components/basic_table/loading_table_body.tsx +++ /dev/null @@ -1,53 +0,0 @@ -import React, { Component } from 'react'; -import { EuiTableBody } from '../table'; - -export class LoadingTableBody extends Component<{}> { - private cleanups: Array<() => void>; - private tbody: HTMLTableSectionElement | null; - - constructor(props: {}) { - super(props); - this.cleanups = []; - this.tbody = null; - } - - componentDidMount() { - const listener = (event: Event) => { - event.stopPropagation(); - event.preventDefault(); - }; - [ - 'mousedown', - 'mouseup', - 'mouseover', - 'mouseout', - 'mouseenter', - 'mouseleave', - 'click', - 'dblclick', - 'keydown', - 'keyup', - 'keypress', - ].forEach(event => { - this.tbody!.addEventListener(event, listener, true); - this.cleanups.push(() => - this.tbody!.removeEventListener(event, listener) - ); - }); - } - - componentWillUnmount() { - this.cleanups.forEach(cleanup => cleanup()); - } - - render() { - return ( - { - this.tbody = tbody; - }}> - {this.props.children} - - ); - } -} From 7942eee259c6c7a6a2b8b67a13163330768f4af9 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 13 Jan 2020 11:50:50 -0700 Subject: [PATCH 2/4] Updated first table example to test loading toggle --- src-docs/src/views/tables/basic/basic.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src-docs/src/views/tables/basic/basic.js b/src-docs/src/views/tables/basic/basic.js index bd104ed0918..75cff3ff0b5 100644 --- a/src-docs/src/views/tables/basic/basic.js +++ b/src-docs/src/views/tables/basic/basic.js @@ -6,6 +6,7 @@ import { EuiBasicTable, EuiLink, EuiHealth, + EuiPopover, } from '../../../../../src/components'; /* @@ -32,7 +33,22 @@ Example country object: const store = createDataStore(); +function CountryColumn({ country }) { + const [isOpen, setIsOpen] = React.useState(false); + + return ( + setIsOpen(false)} + button={ setIsOpen(!isOpen)}>{country}}> + Country: {country} + + ); +} + export const Table = () => { + const [isLoading, setIsLoading] = React.useState(false); + window.setIsLoading = setIsLoading; const columns = [ { field: 'firstName', @@ -82,7 +98,7 @@ export const Table = () => { name: 'Nationality', render: countryCode => { const country = store.getCountry(countryCode); - return `${country.flag} ${country.name}`; + return ; }, }, { @@ -120,6 +136,7 @@ export const Table = () => { return ( Date: Mon, 13 Jan 2020 13:50:42 -0700 Subject: [PATCH 3/4] Revert "Updated first table example to test loading toggle" This reverts commit 7942eee259c6c7a6a2b8b67a13163330768f4af9. --- src-docs/src/views/tables/basic/basic.js | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src-docs/src/views/tables/basic/basic.js b/src-docs/src/views/tables/basic/basic.js index 75cff3ff0b5..bd104ed0918 100644 --- a/src-docs/src/views/tables/basic/basic.js +++ b/src-docs/src/views/tables/basic/basic.js @@ -6,7 +6,6 @@ import { EuiBasicTable, EuiLink, EuiHealth, - EuiPopover, } from '../../../../../src/components'; /* @@ -33,22 +32,7 @@ Example country object: const store = createDataStore(); -function CountryColumn({ country }) { - const [isOpen, setIsOpen] = React.useState(false); - - return ( - setIsOpen(false)} - button={ setIsOpen(!isOpen)}>{country}}> - Country: {country} - - ); -} - export const Table = () => { - const [isLoading, setIsLoading] = React.useState(false); - window.setIsLoading = setIsLoading; const columns = [ { field: 'firstName', @@ -98,7 +82,7 @@ export const Table = () => { name: 'Nationality', render: countryCode => { const country = store.getCountry(countryCode); - return ; + return `${country.flag} ${country.name}`; }, }, { @@ -136,7 +120,6 @@ export const Table = () => { return ( Date: Mon, 13 Jan 2020 13:52:44 -0700 Subject: [PATCH 4/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 633994ff8a2..cad6ace7e59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - Changed `EuiRadio` and `EuiCheckbox` labels to be `inline-block` ([#2739](https://github.com/elastic/eui/pull/2739)) - Fixed `EuiCheckboxGroup`'s `options` type to fully extend the `EuiCheckbox` type ([#2739](https://github.com/elastic/eui/pull/2739)) +- Fixed `EuiBasicTable` & `EuiInMemoryTable` to not un- and re-mount rows when toggling `loading` prop ([#2754](https://github.com/elastic/eui/pull/2754)) ## [`18.0.0`](https://github.com/elastic/eui/tree/v18.0.0)