From 2cbb72a2d5c5013093dfbc94008f9bb15ca93972 Mon Sep 17 00:00:00 2001 From: ushkal <44223690+ushkal@users.noreply.github.com> Date: Wed, 19 Dec 2018 13:17:01 +0100 Subject: [PATCH] fix(react-grid): prevent animation flicker on column visibility change (#1674) --- packages/dx-react-core/src/index.js | 2 + packages/dx-react-core/src/utils/ref-type.js | 6 + .../src/templates/table.jsx | 9 +- .../src/templates/table.test.jsx | 6 +- .../src/templates/table.jsx | 5 +- .../src/templates/table.jsx | 29 ++-- .../src/templates/table.test.jsx | 23 ++- .../src/components/table-layout.jsx | 25 +++- .../src/components/table-layout.test.jsx | 135 +++++++++++++++--- .../virtual-table-layout.test.jsx.snap | 15 ++ .../table-layout/static-table-layout.jsx | 4 + .../table-layout/static-table-layout.test.jsx | 1 + .../table-layout/virtual-table-layout.jsx | 11 +- .../virtual-table-layout.test.jsx | 6 +- 14 files changed, 229 insertions(+), 48 deletions(-) create mode 100644 packages/dx-react-core/src/utils/ref-type.js diff --git a/packages/dx-react-core/src/index.js b/packages/dx-react-core/src/index.js index 99cb3aeddf..7c1f0b86a3 100644 --- a/packages/dx-react-core/src/index.js +++ b/packages/dx-react-core/src/index.js @@ -19,3 +19,5 @@ export { connectProps } from './utils/connect-props'; export { createStateHelper } from './utils/state-helper'; export { withComponents } from './utils/with-components'; + +export { RefType } from './utils/ref-type'; diff --git a/packages/dx-react-core/src/utils/ref-type.js b/packages/dx-react-core/src/utils/ref-type.js new file mode 100644 index 0000000000..aec461c0b7 --- /dev/null +++ b/packages/dx-react-core/src/utils/ref-type.js @@ -0,0 +1,6 @@ +/* globals Element */ +import * as PropTypes from 'prop-types'; + +export const RefType = PropTypes.shape({ + current: PropTypes.instanceOf((typeof Element !== 'undefined') ? Element : Object), +}); diff --git a/packages/dx-react-grid-bootstrap3/src/templates/table.jsx b/packages/dx-react-grid-bootstrap3/src/templates/table.jsx index f95569c347..489489f6ba 100644 --- a/packages/dx-react-grid-bootstrap3/src/templates/table.jsx +++ b/packages/dx-react-grid-bootstrap3/src/templates/table.jsx @@ -2,6 +2,7 @@ import * as React from 'react'; import * as PropTypes from 'prop-types'; import classNames from 'classnames'; +import { RefType } from '@devexpress/dx-react-core'; import { ThemeColors } from './layout'; let globalStickyProp; @@ -43,13 +44,16 @@ export class Table extends React.Component { render() { const { - children, use, style, className, ...restProps + children, use, style, className, tableRef, ...restProps } = this.props; const { stickyProp } = this.state; const { backgroundColor } = this.context; return ( { + this.tableRef.current = ref; + tableRef.current = ref; + }} className={classNames('table', className)} style={{ tableLayout: 'fixed', @@ -83,6 +87,7 @@ Table.propTypes = { children: PropTypes.node.isRequired, style: PropTypes.object, className: PropTypes.string, + tableRef: RefType.isRequired, }; Table.defaultProps = { diff --git a/packages/dx-react-grid-bootstrap3/src/templates/table.test.jsx b/packages/dx-react-grid-bootstrap3/src/templates/table.test.jsx index 6db0243b6b..c700db2119 100644 --- a/packages/dx-react-grid-bootstrap3/src/templates/table.test.jsx +++ b/packages/dx-react-grid-bootstrap3/src/templates/table.test.jsx @@ -4,10 +4,12 @@ import { Table } from './table'; import { ThemeColors } from './layout'; describe('Table', () => { + const tableRef = React.createRef(); + it('should pass class to the root element', () => { const tree = mount(( -
+
@@ -20,7 +22,7 @@ describe('Table', () => { it('should pass rest props to the root element', () => { const tree = mount(( - +
diff --git a/packages/dx-react-grid-bootstrap4/src/templates/table.jsx b/packages/dx-react-grid-bootstrap4/src/templates/table.jsx index da84dcfea6..51fbd77032 100644 --- a/packages/dx-react-grid-bootstrap4/src/templates/table.jsx +++ b/packages/dx-react-grid-bootstrap4/src/templates/table.jsx @@ -1,18 +1,20 @@ import * as React from 'react'; import * as PropTypes from 'prop-types'; import classNames from 'classnames'; +import { RefType } from '@devexpress/dx-react-core'; import { BodyColorContext } from './layout'; // eslint-disable-next-line react/prefer-stateless-function export class Table extends React.Component { render() { const { - children, use, style, className, ...restProps + children, use, style, className, tableRef, ...restProps } = this.props; const backgroundColor = this.context; return ( ({ @@ -29,20 +31,22 @@ const styles = theme => ({ }); const TableBase = ({ - children, classes, className, use, + children, classes, className, use, tableRef, ...restProps }) => ( - - {children} - + + + {children} + + ); TableBase.propTypes = { @@ -50,6 +54,7 @@ TableBase.propTypes = { children: PropTypes.node.isRequired, classes: PropTypes.object.isRequired, className: PropTypes.string, + tableRef: RefType.isRequired, }; TableBase.defaultProps = { diff --git a/packages/dx-react-grid-material-ui/src/templates/table.test.jsx b/packages/dx-react-grid-material-ui/src/templates/table.test.jsx index 550270b76f..b92f21bb16 100644 --- a/packages/dx-react-grid-material-ui/src/templates/table.test.jsx +++ b/packages/dx-react-grid-material-ui/src/templates/table.test.jsx @@ -3,12 +3,15 @@ import { createShallow, getClasses } from '@material-ui/core/test-utils'; import { Table } from './table'; describe('Table', () => { + const defaultProps = { + tableRef: React.createRef(), + }; let shallow; let classes; beforeAll(() => { shallow = createShallow({ dive: true }); classes = getClasses( -
+
, ); @@ -16,26 +19,34 @@ describe('Table', () => { it('should pass the className prop to the root element', () => { const tree = shallow(( - +
)); - expect(tree.is(`.${classes.table}`)) + const table = tree.childAt(0); + expect(table.is(`.${classes.table}`)) .toBeTruthy(); - expect(tree.is('.custom-class')) + expect(table.is('.custom-class')) .toBeTruthy(); }); it('should pass rest props to the root element', () => { const tree = shallow(( - +
)); - expect(tree.props().data) + const table = tree.childAt(0); + expect(table.props().data) .toMatchObject({ a: 1 }); }); }); diff --git a/packages/dx-react-grid/src/components/table-layout.jsx b/packages/dx-react-grid/src/components/table-layout.jsx index 560d5d9d6e..dd7c63a709 100644 --- a/packages/dx-react-grid/src/components/table-layout.jsx +++ b/packages/dx-react-grid/src/components/table-layout.jsx @@ -2,7 +2,6 @@ import * as React from 'react'; import * as PropTypes from 'prop-types'; -import { findDOMNode } from 'react-dom'; import { getAnimations, filterActiveAnimations, @@ -19,19 +18,38 @@ export class TableLayout extends React.PureComponent { }; this.animations = new Map(); + this.savedScrolldWidth = {}; + this.tableRef = React.createRef(); } componentDidUpdate(prevProps) { const { columns } = this.props; const { columns: prevColumns } = prevProps; - // eslint-disable-next-line react/no-find-dom-node - const tableWidth = findDOMNode(this).scrollWidth; + const tableWidth = this.getTableWidth(prevColumns, columns); this.animations = getAnimations(prevColumns, columns, tableWidth, this.animations); + cancelAnimationFrame(this.raf); this.raf = requestAnimationFrame(this.processAnimationFrame.bind(this)); } + getTableWidth(prevColumns, columns) { + const { offsetWidth, scrollWidth } = this.tableRef.current; + const { animationState } = this.state; + + const widthChanged = this.savedOffsetWidth !== offsetWidth + || !this.savedScrolldWidth[columns.length]; + const columnCountChanged = columns.length !== prevColumns.length; + + if (columnCountChanged || (widthChanged && !animationState.size)) { + this.savedScrolldWidth = {}; + this.savedScrolldWidth[columns.length] = scrollWidth; + this.savedOffsetWidth = offsetWidth; + } + + return this.savedScrolldWidth[columns.length]; + } + getColumns() { const { columns } = this.props; const { animationState } = this.state; @@ -83,6 +101,7 @@ export class TableLayout extends React.PureComponent { return ( ({ findDOMNode: jest.fn(() => ({ scrollWidth: 300, + offsetWidth: 200, })), })); jest.mock('@devexpress/dx-grid-core', () => ({ @@ -76,21 +78,32 @@ describe('TableLayout', () => { window.requestAnimationFrame = originalRaf; }); + const layoutComponent = ({ tableRef }) => { + // eslint-disable-next-line no-param-reassign, react/no-find-dom-node + tableRef.current = findDOMNode(); + return null; + }; + const columns = [ + { key: 'a', column: { name: 'a' }, width: 100 }, + { key: 'b', column: { name: 'b' } }, + ]; + const animationDefaultProps = { + ...defaultProps, + columns, + layoutComponent, + minColumnWidth: 100, + }; + it('should be updated on the "columns" property change', () => { filterActiveAnimations.mockImplementation(() => new Map()); evalAnimations.mockImplementation(() => new Map()); - const columns = [ - { key: 'a', column: { name: 'a' }, width: 100 }, - { key: 'b', column: { name: 'b' } }, - ]; + const nextColumns = [columns[1], columns[0]]; - const tree = shallow(( + const tree = mount(( )); tree.setProps({ columns: nextColumns }); @@ -103,10 +116,6 @@ describe('TableLayout', () => { }); it('should start on the "columns" property change', () => { - const columns = [ - { key: 'a', column: { name: 'a' }, width: 100 }, - { key: 'b', column: { name: 'b' } }, - ]; const nextColumns = [columns[1], columns[0]]; const animations = new Map([ ['a', { left: { from: 200, to: 0 } }], @@ -115,11 +124,9 @@ describe('TableLayout', () => { filterActiveAnimations.mockImplementation(() => animations); - const tree = shallow(( + const tree = mount(( )); tree.setProps({ columns: nextColumns }); @@ -132,5 +139,99 @@ describe('TableLayout', () => { expect(evalAnimations) .toHaveBeenCalledWith(animations); }); + + describe('cache table width', () => { + const nextColumns = columns.slice(0, 2); + const tableDimensions = { scrollWidth: 300, offsetWidth: 200 }; + + it('should not reset width if scroll width changed', () => { + const tree = mount(( + + )); + findDOMNode + .mockReturnValueOnce(tableDimensions) + .mockReturnValueOnce({ ...tableDimensions, scrollWidth: 400 }) + .mockReturnValue(tableDimensions); + filterActiveAnimations.mockImplementation(() => new Map()); + + tree.setProps({ columns: nextColumns }); + rafCallback(); + tree.setProps({ columns: nextColumns.slice() }); + tree.update(); + rafCallback(); + + expect(getAnimations).toHaveBeenCalledTimes(2); + expect(getAnimations) + .toHaveBeenLastCalledWith(expect.anything(), expect.anything(), 300, new Map()); + }); + + it('should reset width if offset width changed', () => { + const tree = mount(( + + )); + findDOMNode + .mockReturnValueOnce(tableDimensions) + .mockReturnValueOnce({ scrollWidth: 400, offsetWidth: 300 }) + .mockReturnValue(tableDimensions); + filterActiveAnimations.mockImplementation(() => new Map()); + + tree.setProps({ columns: nextColumns }); + rafCallback(); + tree.setProps({ columns: nextColumns.slice() }); + rafCallback(); + + expect(getAnimations).toHaveBeenCalledTimes(2); + expect(getAnimations) + .toHaveBeenLastCalledWith(expect.anything(), expect.anything(), 400, new Map()); + }); + + it('should reset width if column count changed', () => { + const tree = mount(( + + )); + findDOMNode + .mockReturnValueOnce(tableDimensions) + .mockReturnValueOnce({ scrollWidth: 400, offsetWidth: 200 }) + .mockReturnValue(tableDimensions); + filterActiveAnimations.mockImplementation(() => new Map()); + + tree.setProps({ columns: nextColumns }); + rafCallback(); + tree.setProps({ columns: columns.slice(1) }); + rafCallback(); + + expect(getAnimations).toHaveBeenCalledTimes(2); + expect(getAnimations) + .toHaveBeenLastCalledWith(expect.anything(), expect.anything(), 400, new Map()); + }); + + it('should not reset width if animations not finished', () => { + const tree = mount(( + + )); + findDOMNode + .mockReturnValueOnce(tableDimensions) + .mockReturnValueOnce({ scrollWidth: 400, offsetWidth: 200 }) + .mockReturnValue(tableDimensions); + filterActiveAnimations.mockImplementation(() => new Map()); + + tree.setProps({ columns: nextColumns }); + rafCallback(); + tree.setProps({ columns: columns.slice(0, 1) }); + rafCallback(); + + expect(getAnimations).toHaveBeenCalledTimes(2); + expect(getAnimations) + .toHaveBeenLastCalledWith(expect.anything(), expect.anything(), 400, new Map()); + }); + }); }); }); diff --git a/packages/dx-react-grid/src/components/table-layout/__snapshots__/virtual-table-layout.test.jsx.snap b/packages/dx-react-grid/src/components/table-layout/__snapshots__/virtual-table-layout.test.jsx.snap index 849cfaafb3..c9a4777049 100644 --- a/packages/dx-react-grid/src/components/table-layout/__snapshots__/virtual-table-layout.test.jsx.snap +++ b/packages/dx-react-grid/src/components/table-layout/__snapshots__/virtual-table-layout.test.jsx.snap @@ -16,6 +16,11 @@ exports[`VirtualTableLayout should render correct layout 1`] = ` "minWidth": "400px", } } + tableRef={ + Object { + "current": null, + } + } > @@ -75,6 +78,7 @@ StaticTableLayout.propTypes = { rowComponent: PropTypes.func.isRequired, cellComponent: PropTypes.func.isRequired, getCellColSpan: PropTypes.func.isRequired, + tableRef: RefType.isRequired, }; StaticTableLayout.defaultProps = { diff --git a/packages/dx-react-grid/src/components/table-layout/static-table-layout.test.jsx b/packages/dx-react-grid/src/components/table-layout/static-table-layout.test.jsx index f3ce5f7480..16678f0e4d 100644 --- a/packages/dx-react-grid/src/components/table-layout/static-table-layout.test.jsx +++ b/packages/dx-react-grid/src/components/table-layout/static-table-layout.test.jsx @@ -22,6 +22,7 @@ const defaultProps = { cellComponent: () => null, rowComponent: () => null, getCellColSpan: () => 1, + tableRef: React.createRef(), }; describe('StaticTableLayout', () => { diff --git a/packages/dx-react-grid/src/components/table-layout/virtual-table-layout.jsx b/packages/dx-react-grid/src/components/table-layout/virtual-table-layout.jsx index 6019b9db10..6fb079f7e6 100644 --- a/packages/dx-react-grid/src/components/table-layout/virtual-table-layout.jsx +++ b/packages/dx-react-grid/src/components/table-layout/virtual-table-layout.jsx @@ -2,7 +2,7 @@ import * as React from 'react'; import { findDOMNode } from 'react-dom'; import * as PropTypes from 'prop-types'; import { isEdgeBrowser } from '@devexpress/dx-core'; -import { Sizer, RefHolder } from '@devexpress/dx-react-core'; +import { Sizer, RefHolder, RefType } from '@devexpress/dx-react-core'; import { getCollapsedGrid, TABLE_FLEX_TYPE, @@ -181,18 +181,21 @@ export class VirtualTableLayout extends React.PureComponent { this.setState({ width, height }); } - renderRowsBlock(name, collapsedGrid, Table, Body, marginBottom) { + renderRowsBlock(name, collapsedGrid, Table, Body, blockRef, marginBottom) { const { minWidth, rowComponent: Row, cellComponent: Cell, } = this.props; + const tableRef = blockRef || React.createRef(); + return ( this.registerBlockRef(name, ref)} >
{!!headerRows.length && this.renderRowsBlock('header', collapsedHeaderGrid, HeadTable, Head)} - {this.renderRowsBlock('body', collapsedBodyGrid, Table, Body, Math.max(0, height - headerHeight - bodyHeight - footerHeight))} + {this.renderRowsBlock('body', collapsedBodyGrid, Table, Body, tableRef, Math.max(0, height - headerHeight - bodyHeight - footerHeight))} {!!footerRows.length && this.renderRowsBlock('footer', collapsedFooterGrid, FootTable, Footer)} ); @@ -338,6 +342,7 @@ VirtualTableLayout.propTypes = { containerComponent: PropTypes.func.isRequired, estimatedRowHeight: PropTypes.number.isRequired, getCellColSpan: PropTypes.func.isRequired, + tableRef: RefType.isRequired, }; VirtualTableLayout.defaultProps = { diff --git a/packages/dx-react-grid/src/components/table-layout/virtual-table-layout.test.jsx b/packages/dx-react-grid/src/components/table-layout/virtual-table-layout.test.jsx index e3d58be277..f1f891434c 100644 --- a/packages/dx-react-grid/src/components/table-layout/virtual-table-layout.test.jsx +++ b/packages/dx-react-grid/src/components/table-layout/virtual-table-layout.test.jsx @@ -26,6 +26,7 @@ jest.mock('./column-group', () => ({ jest.mock('@devexpress/dx-react-core', () => { const { Component } = require.requireActual('react'); return { + ...require.requireActual('@devexpress/dx-react-core'), // eslint-disable-next-line react/prefer-stateless-function Sizer: class extends Component { componentDidMount() { @@ -76,13 +77,14 @@ const defaultProps = { { key: 9 }, ], containerComponent: props =>
, - headTableComponent: props =>
, - tableComponent: props =>
, + headTableComponent: ({ tableRef, ...props }) =>
, + tableComponent: ({ tableRef, ...props }) =>
, headComponent: props => , bodyComponent: props => , rowComponent: () => null, cellComponent: () => null, getCellColSpan: () => 1, + tableRef: React.createRef(), }; describe('VirtualTableLayout', () => {