From bbeb122af1a994946401ec0beb298ddc164ab98d Mon Sep 17 00:00:00 2001 From: Allen Date: Tue, 24 Oct 2017 00:56:59 -0500 Subject: [PATCH] fix wrong higher order wrapping (#112) * fix wrong higher order wrapping * fix tests for wrong wrapper design --- .../src/cell-edit/wrapper.js | 8 ++- .../react-bootstrap-table2/src/container.js | 71 +++++-------------- .../src/row-selection/wrapper.js | 7 +- .../src/sort/wrapper.js | 8 ++- .../src/table-factory.js | 27 +++++++ .../test/cell-edit/wrapper.test.js | 34 +++++---- .../test/container.test.js | 61 +++++++++++++--- .../test/row-selection/wrapper.test.js | 14 ++-- .../test/sort/wrapper.test.js | 12 ++-- 9 files changed, 146 insertions(+), 96 deletions(-) create mode 100644 packages/react-bootstrap-table2/src/table-factory.js diff --git a/packages/react-bootstrap-table2/src/cell-edit/wrapper.js b/packages/react-bootstrap-table2/src/cell-edit/wrapper.js index 20f68c0..da6963f 100644 --- a/packages/react-bootstrap-table2/src/cell-edit/wrapper.js +++ b/packages/react-bootstrap-table2/src/cell-edit/wrapper.js @@ -1,9 +1,11 @@ /* eslint arrow-body-style: 0 */ /* eslint react/prop-types: 0 */ -import React, { Component } from 'react'; +import { Component } from 'react'; import PropTypes from 'prop-types'; import _ from '../utils'; +import { cellEditElement } from '../table-factory'; + class CellEditWrapper extends Component { constructor(props) { super(props); @@ -88,7 +90,8 @@ class CellEditWrapper extends Component { } render() { - return React.cloneElement(this.props.elem, { + return cellEditElement({ + ...this.props, onCellUpdate: this.handleCellUpdate, onStartEditing: this.startEditing, onEscapeEditing: this.escapeEditing, @@ -98,7 +101,6 @@ class CellEditWrapper extends Component { } CellEditWrapper.propTypes = { - elem: PropTypes.element.isRequired, onUpdateCell: PropTypes.func.isRequired }; diff --git a/packages/react-bootstrap-table2/src/container.js b/packages/react-bootstrap-table2/src/container.js index d8d964a..e4bf9a4 100644 --- a/packages/react-bootstrap-table2/src/container.js +++ b/packages/react-bootstrap-table2/src/container.js @@ -1,16 +1,18 @@ -/* eslint arrow-body-style: 0 */ -/* eslint react/jsx-no-bind: 0 */ /* eslint no-return-assign: 0 */ /* eslint react/prop-types: 0 */ import React, { Component } from 'react'; import Store from './store/base'; -import SortWrapper from './sort/wrapper'; -import CellEditWrapper from './cell-edit/wrapper'; -import RowSelectionWrapper from './row-selection/wrapper'; + +import { + wrapWithCellEdit, + wrapWithSelection, + wrapWithSort +} from './table-factory'; + import _ from './utils'; -const withDataStore = (Base) => { - return class BootstrapTableContainer extends Component { +const withDataStore = Base => + class BootstrapTableContainer extends Component { constructor(props) { super(props); this.store = new Store(props); @@ -46,61 +48,26 @@ const withDataStore = (Base) => { return false; } - renderCellEdit(elem) { - return ( - this.cellEditWrapper = node } - elem={ elem } - onUpdateCell={ this.handleUpdateCell } - /> - ); - } - - renderRowSelection(elem) { - return ( - - ); - } - - renderSort(elem) { - return ( - - ); - } - render() { const baseProps = { ...this.props, store: this.store }; - let element = React.createElement(Base, baseProps); - - // @TODO - // the logic of checking sort is enable or not should be placed in the props resolver.. - // but currently, I've no idea to refactoring this - if (this.props.columns.filter(col => col.sort).length > 0) { - element = this.renderSort(element); - } - - if (this.props.selectRow) { - element = this.renderRowSelection(element); - } - if (this.props.cellEdit) { - element = this.renderCellEdit(element); + return wrapWithCellEdit({ + ref: node => this.cellEditWrapper = node, + onUpdateCell: this.handleUpdateCell, + ...baseProps + }); + } else if (this.props.selectRow) { + return wrapWithSelection(baseProps); + } else if (this.props.columns.filter(col => col.sort).length > 0) { + return wrapWithSort(baseProps); } - return element; + return React.createElement(Base, baseProps); } }; -}; export default withDataStore; diff --git a/packages/react-bootstrap-table2/src/row-selection/wrapper.js b/packages/react-bootstrap-table2/src/row-selection/wrapper.js index 97909f2..dad9d6e 100644 --- a/packages/react-bootstrap-table2/src/row-selection/wrapper.js +++ b/packages/react-bootstrap-table2/src/row-selection/wrapper.js @@ -1,7 +1,8 @@ /* eslint arrow-body-style: 0 */ /* eslint react/prop-types: 0 */ -import React, { Component } from 'react'; +import { Component } from 'react'; import PropTypes from 'prop-types'; +import { selectionElement } from '../table-factory'; import Const from '../const'; @@ -65,7 +66,8 @@ class RowSelectionWrapper extends Component { } render() { - return React.cloneElement(this.props.elem, { + return selectionElement({ + ...this.props, onRowSelect: this.handleRowSelect, onAllRowsSelect: this.handleAllRowsSelect }); @@ -73,7 +75,6 @@ class RowSelectionWrapper extends Component { } RowSelectionWrapper.propTypes = { - elem: PropTypes.element.isRequired, store: PropTypes.object.isRequired }; diff --git a/packages/react-bootstrap-table2/src/sort/wrapper.js b/packages/react-bootstrap-table2/src/sort/wrapper.js index e9643eb..af16eed 100644 --- a/packages/react-bootstrap-table2/src/sort/wrapper.js +++ b/packages/react-bootstrap-table2/src/sort/wrapper.js @@ -1,9 +1,11 @@ /* eslint arrow-body-style: 0 */ /* eslint react/prop-types: 0 */ /* eslint no-return-assign: 0 */ -import React, { Component } from 'react'; +import { Component } from 'react'; import PropTypes from 'prop-types'; +import { sortableElement } from '../table-factory'; + class SortWrapper extends Component { constructor(props) { super(props); @@ -20,7 +22,8 @@ class SortWrapper extends Component { } render() { - return React.cloneElement(this.props.elem, { + return sortableElement({ + ...this.props, ref: node => this.table = node, onSort: this.handleSort }); @@ -28,7 +31,6 @@ class SortWrapper extends Component { } SortWrapper.propTypes = { - elem: PropTypes.element.isRequired, store: PropTypes.object.isRequired }; diff --git a/packages/react-bootstrap-table2/src/table-factory.js b/packages/react-bootstrap-table2/src/table-factory.js new file mode 100644 index 0000000..0eddc84 --- /dev/null +++ b/packages/react-bootstrap-table2/src/table-factory.js @@ -0,0 +1,27 @@ +import React from 'react'; + +import BootstrapTable from './bootstrap-table'; + +import SortWrapper from './sort/wrapper'; +import RowSelectionWrapper from './row-selection/wrapper'; +import CellEditWrapper from './cell-edit/wrapper'; + + +export const wrapWithCellEdit = props => + React.createElement(CellEditWrapper, { ...props }); + +export const wrapWithSelection = props => + React.createElement(RowSelectionWrapper, { ...props }); + +export const wrapWithSort = props => + React.createElement(SortWrapper, { ...props }); + +export const pureTable = props => + React.createElement(BootstrapTable, { ...props }); + + +export const sortableElement = props => pureTable(props); + +export const selectionElement = props => wrapWithSort(props); + +export const cellEditElement = props => wrapWithSelection(props); diff --git a/packages/react-bootstrap-table2/test/cell-edit/wrapper.test.js b/packages/react-bootstrap-table2/test/cell-edit/wrapper.test.js index ac504fa..49ebd22 100644 --- a/packages/react-bootstrap-table2/test/cell-edit/wrapper.test.js +++ b/packages/react-bootstrap-table2/test/cell-edit/wrapper.test.js @@ -8,7 +8,6 @@ import CellEditWrapper from '../../src/cell-edit/wrapper'; describe('CellEditWrapper', () => { let wrapper; - let elem; const columns = [{ dataField: 'id', @@ -35,12 +34,13 @@ describe('CellEditWrapper', () => { const store = new Store({ data, keyField }); beforeEach(() => { - elem = React.createElement(BootstrapTable, { data, cellEdit, columns, keyField, store }); wrapper = shallow( ); @@ -58,7 +58,7 @@ describe('CellEditWrapper', () => { expect(wrapper.state().editing).toBeFalsy(); }); - it('should inject correct props to elem', () => { + it('should inject correct props to base component', () => { expect(wrapper.props().onCellUpdate).toBeDefined(); expect(wrapper.props().onStartEditing).toBeDefined(); expect(wrapper.props().onEscapeEditing).toBeDefined(); @@ -74,12 +74,13 @@ describe('CellEditWrapper', () => { describe('and cellEdit.editing is false', () => { beforeEach(() => { - elem = React.createElement(BootstrapTable, { data, cellEdit, columns, keyField, store }); wrapper = shallow( ); @@ -104,12 +105,13 @@ describe('CellEditWrapper', () => { const cidx = 2; beforeEach(() => { - elem = React.createElement(BootstrapTable, { data, cellEdit, columns, keyField, store }); wrapper = shallow( ); @@ -175,8 +177,10 @@ describe('CellEditWrapper', () => { wrapper = shallow( ); @@ -204,8 +208,10 @@ describe('CellEditWrapper', () => { wrapper = shallow( ); @@ -229,8 +235,10 @@ describe('CellEditWrapper', () => { wrapper = shallow( ); @@ -249,8 +257,10 @@ describe('CellEditWrapper', () => { wrapper = shallow( ); diff --git a/packages/react-bootstrap-table2/test/container.test.js b/packages/react-bootstrap-table2/test/container.test.js index 371441f..dc6cbd2 100644 --- a/packages/react-bootstrap-table2/test/container.test.js +++ b/packages/react-bootstrap-table2/test/container.test.js @@ -3,6 +3,9 @@ import sinon from 'sinon'; import { shallow } from 'enzyme'; import BootstrapTable from '../src'; +import SortWrapper from '../src/sort/wrapper'; +import CellEditWrapper from '../src/cell-edit/wrapper'; +import RowSelectionWrapper from '../src/row-selection/wrapper'; describe('withDataStore', () => { let wrapper; @@ -44,8 +47,7 @@ describe('withDataStore', () => { }); }); - describe('when cellEdit is defined', () => { - const spy = jest.spyOn(BootstrapTable.prototype, 'renderCellEdit'); + describe('when cellEdit prop is defined', () => { const cellEdit = { mode: 'click' }; @@ -61,15 +63,14 @@ describe('withDataStore', () => { ); }); - it('should calling renderCellEdit function', () => { - expect(spy).toHaveBeenCalled(); + it('should render CellEditWrapper component successfully', () => { + const component = wrapper.find(CellEditWrapper); + expect(component.length).toBe(1); }); - it('should injecting correct props', () => { - expect(wrapper.props().keyField).toEqual('id'); - expect(wrapper.props().cellEdit).toEqual(cellEdit); - expect(wrapper.props().elem).toBeDefined(); - expect(wrapper.props().onUpdateCell).toBeDefined(); + it('should injecting correct props to CellEditWrapper', () => { + const component = wrapper.find(CellEditWrapper); + expect(component.props().onUpdateCell).toBeDefined(); }); describe('for handleUpdateCell function', () => { @@ -129,4 +130,46 @@ describe('withDataStore', () => { describe.skip('when cellEdit.onUpdate callback is define and which return a Promise', () => {}); }); }); + + describe('when selectRow prop is defined', () => { + const selectRow = { + mode: 'checkbox' + }; + + beforeEach(() => { + wrapper = shallow( + + ); + }); + + it('should render RowSelectionWrapper component successfully', () => { + expect(wrapper.find(RowSelectionWrapper).length).toBe(1); + }); + }); + + describe('when any column.sort is defined', () => { + beforeEach(() => { + const columnsWithSort = [{ + dataField: keyField, + text: 'ID', + sort: true + }]; + wrapper = shallow( + + ); + }); + + it('should render SortWrapper component successfully', () => { + expect(wrapper.find(SortWrapper).length).toBe(1); + }); + }); }); diff --git a/packages/react-bootstrap-table2/test/row-selection/wrapper.test.js b/packages/react-bootstrap-table2/test/row-selection/wrapper.test.js index 1404259..9ab18b6 100644 --- a/packages/react-bootstrap-table2/test/row-selection/wrapper.test.js +++ b/packages/react-bootstrap-table2/test/row-selection/wrapper.test.js @@ -1,5 +1,4 @@ import React from 'react'; -import sinon from 'sinon'; import { shallow } from 'enzyme'; import Store from '../../src/store/base'; @@ -8,7 +7,6 @@ import RowSelectionWrapper from '../../src/row-selection/wrapper'; describe('RowSelectionWrapper', () => { let wrapper; - let elem; const columns = [{ dataField: 'id', @@ -35,14 +33,13 @@ describe('RowSelectionWrapper', () => { const store = new Store({ data, keyField }); beforeEach(() => { - elem = React.createElement(BootstrapTable, { data, selectRow, columns, keyField, store }); wrapper = shallow( ); }); @@ -57,7 +54,7 @@ describe('RowSelectionWrapper', () => { expect(wrapper.state().selectedRowKeys.length).toEqual(0); }); - it('should inject correct props to elem', () => { + it('should inject correct props to base component', () => { expect(wrapper.props().onRowSelect).toBeDefined(); expect(wrapper.props().onAllRowsSelect).toBeDefined(); }); @@ -81,14 +78,13 @@ describe('RowSelectionWrapper', () => { beforeEach(() => { selectRow.mode = 'checkbox'; - elem = React.createElement(BootstrapTable, { data, selectRow, columns, keyField, store }); wrapper = shallow( ); }); diff --git a/packages/react-bootstrap-table2/test/sort/wrapper.test.js b/packages/react-bootstrap-table2/test/sort/wrapper.test.js index e0a9ed4..f6f9613 100644 --- a/packages/react-bootstrap-table2/test/sort/wrapper.test.js +++ b/packages/react-bootstrap-table2/test/sort/wrapper.test.js @@ -9,7 +9,6 @@ import SortWrapper from '../../src/sort/wrapper'; describe('SortWrapper', () => { let wrapper; - let elem; const columns = [{ dataField: 'id', @@ -33,10 +32,11 @@ describe('SortWrapper', () => { let store = new Store({ data, keyField }); beforeEach(() => { - elem = React.createElement(BootstrapTable, { data, columns, keyField, store }); wrapper = shallow( ); @@ -47,7 +47,7 @@ describe('SortWrapper', () => { expect(wrapper.find(BootstrapTable)).toBeDefined(); }); - it('should inject correct props to elem', () => { + it('should inject correct props to base component', () => { expect(wrapper.props().onSort).toBeDefined(); }); @@ -58,7 +58,9 @@ describe('SortWrapper', () => { store = new Store({ data, keyField }); wrapper = mount( );